Bug 183742 – [Extra zoom mode] Require fullscreen for video playback

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Eric Carlson]

Require fullscreen for video playback

[Eric Carlson]

<rdar://problem/38235862>

[Eric Carlson]

Created attachment 336042 [details]
Patch

[Jon Lee]

Comment on attachment 336042 [details]
Patch

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

> Source/WebCore/Modules/modern-media-controls/media/playback-support.js:52
> +            this.control.visible = true;

This may cause a problem when a compact player is playing inline. Can we differentiate between when we are in extra zoom mode?

[Eric Carlson]

Created attachment 336044 [details]
Patch

[Jer Noble]

Comment on attachment 336044 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:5272
> -            if (m_mediaSession->requiresFullscreenForVideoPlayback(*this) && !isFullscreen())
> +            if (m_mediaSession->requiresFullscreenForVideoPlayback(*this) && !isFullscreen()) {
>                  enterFullscreen();
> +#if ENABLE(EXTRA_ZOOM_MODE)
> +                return;
> +#endif
> +            }

Why not just make the early return unconditional? Then this could be moved outside the if() statement; mitigating any side effects to things like "setDisplayMode()" and "clientWillBeginPlayback()".

[Eric Carlson]

Created attachment 336049 [details]
Patch

[Eric Carlson]

Created attachment 336060 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 336060 [details]
Patch

Clearing flags on attachment: 336060

Committed r229714: <https://trac.webkit.org/changeset/229714>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.