[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 #47 from Darin Adler <darin at apple.com>  2013-06-07 17:47:05 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
> +#if !defined(ENABLE_TOGGLE_VIDEO_FULLSCREEN)
> +#define ENABLE_TOGGLE_VIDEO_FULLSCREEN 1
> +#endif

Does not seem right to have this be a feature flag.

> Source/WebCore/html/HTMLMediaElement.cpp:4375
> +#if ENABLE(TOGGLE_VIDEO_FULLSCREEN)
> +void HTMLMediaElement::toggleFullscreenState()
> +{
> +    LOG(Media, "HTMLMediaElement::toggleFullscreenState - isFullscreen() is %s", boolString(isFullscreen()));
> +    
> +    if (isFullscreen())
> +        exitFullscreen();
> +    else
> +        enterFullscreen();
> +}
> +#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.

> Source/WebCore/platform/ContextMenuItem.h:163
> +#if ENABLE(TOGGLE_VIDEO_FULLSCREEN)
> +        ContextMenuItemTagToggleVideoFullscreen,
> +#else
>          ContextMenuItemTagEnterVideoFullscreen,
> +#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?

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