RESOLVED FIXED 55239
Allow webkitEnterFullScreen from outside user gesture when restrictions are unrestricted
https://bugs.webkit.org/show_bug.cgi?id=55239
Summary Allow webkitEnterFullScreen from outside user gesture when restrictions are u...
Dean Jackson
Reported Friday, February 25, 2011 7:11:38 PM UTC
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>
Attachments
Patch (2.49 KB, patch)
2011-02-25 11:25 PST, Dean Jackson
eric.carlson: review-
Updated patch (4.46 KB, patch)
2011-02-25 12:30 PST, Dean Jackson
eric.carlson: review-
Another update (4.48 KB, patch)
2011-02-25 13:23 PST, Dean Jackson
no flags
Dean Jackson
Comment 1 Friday, February 25, 2011 7:25:29 PM UTC
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 2 Friday, February 25, 2011 8:05:21 PM UTC
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
Comment 3 Friday, February 25, 2011 8:30:01 PM UTC
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 4 Friday, February 25, 2011 8:35:27 PM UTC
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
Comment 5 Friday, February 25, 2011 9:23:03 PM UTC
Created attachment 83866 [details] Another update Made Eric's changes
Dean Jackson
Comment 6 Monday, February 28, 2011 10:33:10 PM UTC
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
Comment 7 Thursday, June 23, 2011 1:47:17 AM UTC
*** Bug 63202 has been marked as a duplicate of this bug. ***
Alexis Menard (darktears)
Comment 8 Thursday, June 23, 2011 11:45:33 AM UTC
Comment on attachment 83866 [details] Another update I believe I can clear the flag it has been landed.
Note You need to log in before you can comment on or make changes to this bug.