Bug 55239

Summary: Allow webkitEnterFullScreen from outside user gesture when restrictions are unrestricted
Product: WebKit Reporter: Dean Jackson <dino>
Component: MediaAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: adamjernst, eric.carlson
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
eric.carlson: review-
Updated patch
eric.carlson: review-
Another update none

Description Dean Jackson 2011-02-25 11:11:38 PST
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>
Comment 1 Dean Jackson 2011-02-25 11:25:29 PST
Created attachment 83850 [details]
Patch

I can't think of a good way to test this. It requires tweaking restrictions on media elements.
Comment 2 Eric Carlson 2011-02-25 12:05:21 PST
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.
Comment 3 Dean Jackson 2011-02-25 12:30:01 PST
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.
Comment 4 Eric Carlson 2011-02-25 12:35:27 PST
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)"
Comment 5 Dean Jackson 2011-02-25 13:23:03 PST
Created attachment 83866 [details]
Another update

Made Eric's changes
Comment 6 Dean Jackson 2011-02-28 14:33:10 PST
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/html/HTMLMediaElement.cpp
	M	Source/WebCore/html/HTMLMediaElement.h
	M	Source/WebCore/html/HTMLVideoElement.cpp
Committed r79922
Comment 7 Adam 2011-06-22 17:47:17 PDT
*** Bug 63202 has been marked as a duplicate of this bug. ***
Comment 8 Alexis Menard (darktears) 2011-06-23 03:45:33 PDT
Comment on attachment 83866 [details]
Another update

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