Bug 75867 - Compilation errors on build-webkit --debug --no-video --no-fullscreen-api on mac.
Summary: Compilation errors on build-webkit --debug --no-video --no-fullscreen-api on ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.6
: P2 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-09 10:48 PST by Pablo Flouret
Modified: 2012-01-15 12:14 PST (History)
5 users (show)

See Also:


Attachments
proposed patch (6.68 KB, patch)
2012-01-09 10:49 PST, Pablo Flouret
no flags Details | Formatted Diff | Diff
don't enable video-track on gtk or blackberry by default (6.60 KB, patch)
2012-01-09 11:30 PST, Pablo Flouret
no flags Details | Formatted Diff | Diff
leave out --no-fullscreen issues. (8.10 KB, patch)
2012-01-10 14:27 PST, Pablo Flouret
no flags Details | Formatted Diff | Diff
fixes from review comments (8.94 KB, patch)
2012-01-13 10:41 PST, Pablo Flouret
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pablo Flouret 2012-01-09 10:48:00 PST
Compilation errors on build-webkit --debug --no-video --no-fullscreen-api on mac.
Comment 1 Pablo Flouret 2012-01-09 10:49:59 PST
Created attachment 121686 [details]
proposed patch
Comment 2 Gustavo Noronha (kov) 2012-01-09 10:58:36 PST
Comment on attachment 121686 [details]
proposed patch

Attachment 121686 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11184590
Comment 3 Pablo Flouret 2012-01-09 11:30:25 PST
Created attachment 121701 [details]
don't enable video-track on gtk or blackberry by default
Comment 4 Philippe Normand 2012-01-10 00:31:01 PST
Comment on attachment 121701 [details]
don't enable video-track on gtk or blackberry by default

View in context: https://bugs.webkit.org/attachment.cgi?id=121701&action=review

> Source/JavaScriptCore/wtf/Platform.h:546
> +#if defined(ENABLE_VIDEO)
> +#define ENABLE_FULLSCREEN_API 1

This is wrong I think. The FullScreen API allows about any HTML element to request full screen display.
Comment 5 Philippe Normand 2012-01-10 00:32:08 PST
What errors do you get exactly?
Comment 6 Pablo Flouret 2012-01-10 14:24:06 PST
My bad, i thought fullscreen depended on video since i usually heard mention of it in that context, i'll just fix the --no-video issues in this bug.
Comment 7 Pablo Flouret 2012-01-10 14:27:05 PST
Created attachment 121911 [details]
leave out --no-fullscreen issues.
Comment 8 Philippe Normand 2012-01-11 00:36:25 PST
Comment on attachment 121911 [details]
leave out --no-fullscreen issues.

View in context: https://bugs.webkit.org/attachment.cgi?id=121911&action=review

Can you please elaborate the ChangeLog entries instead of copy/pasting the same message?

> Source/WebCore/rendering/RenderThemeMac.h:176
> -#endif
> -    
>      virtual bool shouldShowPlaceholderWhenFocused() const;
> +#endif

I might be wrong here, but I think this method was added under the wrong guard in the first place. It's used only in the HTMLTextFormControlElement from what I can tell, which has nothing to do with VIDEO.

> Source/WebKit/mac/WebCoreSupport/WebChromeClient.mm:947
> +void WebChromeClient::fullScreenRendererChanged(RenderBox* renderer)

This method should not be under VIDEO, it is guarded by FULLSCREEN_API in ChromeClient.h.
Comment 9 Pablo Flouret 2012-01-13 10:41:13 PST
Created attachment 122453 [details]
fixes from review comments
Comment 10 WebKit Review Bot 2012-01-15 12:14:47 PST
Comment on attachment 122453 [details]
fixes from review comments

Clearing flags on attachment: 122453

Committed r105032: <http://trac.webkit.org/changeset/105032>
Comment 11 WebKit Review Bot 2012-01-15 12:14:52 PST
All reviewed patches have been landed.  Closing bug.