Bug 55239 – Allow webkitEnterFullScreen from outside user gesture when restrictions are unrestricted

• Component: Media
• Status: RESOLVED FIXED

[Dean Jackson]

webkitEnterFullScreen is protected by a user gesture. If the rateChange restriction is relaxed, full screen should be allowed outside gestures.

Meanwhile, the helper functions that return the restriction info should be moved from private to protected.

<rdar://problem/7648874>

[Dean Jackson]

Created attachment 83850 [details]
Patch

I can't think of a good way to test this. It requires tweaking restrictions on media elements.

[Eric Carlson]

Comment on attachment 83850 [details]
Patch

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

I think it would make more sense to define a new restriction, maybe RequireUserGestureForFullScreen, and have it be set by default but allow a port to clear if it wants to allow unrestricted fullscreen.

> Source/WebCore/html/HTMLVideoElement.cpp:235
> -    if (!isUserGesture || !supportsFullscreen()) {
> +    if ((!isUserGesture && requireUserGestureForRateChange()) || !supportsFullscreen()) {

This will allow any script to trigger fullscreen for any port that doesn't restrict playback to a user gesture - not good.

[Dean Jackson]

Created attachment 83861 [details]
Updated patch

Good point. Added a new restriction, but made the access methods public so it is easier for a port to add/remove restrictions. FullScreen is now protected by default.

[Eric Carlson]

Comment on attachment 83861 [details]
Updated patch

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

> Source/WebCore/html/HTMLMediaElement.h:191
> +    void setRestrictions(BehaviorRestrictions restrictions) { m_restrictions = restrictions; }

setRestrictions" is pretty generic, setBehaviorRestrictions maybe?


> Source/WebCore/html/HTMLVideoElement.cpp:235
> +    if ((!isUserGesture && requireUserGestureForFullScreen()) || !supportsFullscreen()) {

As long as you are making a change anyway, I think this will be slightly clearer if you switch the order of the comparisons: "(requireUserGestureForFullScreen() && !isUserGesture)"

[Dean Jackson]

Created attachment 83866 [details]
Another update

Made Eric's changes

[Dean Jackson]

	M	Source/WebCore/ChangeLog
	M	Source/WebCore/html/HTMLMediaElement.cpp
	M	Source/WebCore/html/HTMLMediaElement.h
	M	Source/WebCore/html/HTMLVideoElement.cpp
Committed r79922

[Adam]

*** Bug 63202 has been marked as a duplicate of this bug. ***

[Alexis Menard (darktears)]

Comment on attachment 83866 [details]
Another update

I believe I can clear the flag it has been landed.