[Webkit-unassigned] [Bug 117220] Allow for toggling fullscreen on <video> elements

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=117220





--- Comment #50 from Ruth Fong <ruthiecftg at gmail.com>  2013-06-07 17:59:54 PST ---
(From update of attachment 204076)
View in context: https://bugs.webkit.org/attachment.cgi?id=204076&action=review

>> Source/WTF/wtf/FeatureDefines.h:757
>> +#endif
> 
> Does not seem right to have this be a feature flag.

There should be some sort of pre-processing checking for parts of this patch (for ports that do want to be able to toggle fullscreen, other parts need to be removed) and defining it as a feature flag seems to be the best way to minimize the amount of code change needed to allow other ports to upgrade their APIs to support toggling fullscreen video.

>> Source/WebCore/html/HTMLMediaElement.cpp:4375
>> +#endif
> 
> This should be in there unconditionally. Whether someone wants this command in the context menu or not should not control whether we even compile this code.

You're right.

>> Source/WebCore/platform/ContextMenuItem.h:163
>> +#endif
> 
> I really don’t understand this. Why is the toggle video and enter video an either/or choice? Why can’t both be possible context menu items?

The toggle video item also encompasses entering into fullscreen; the either/or choice was made to encourage either supporting the existing implementation of fullscreen context menu item or switching to a toggling implementation but not to support both (that would create the side effect of possibly having two fullscreen context menu items).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.