Bug 55239 - Allow webkitEnterFullScreen from outside user gesture when restrictions are unrestricted
Summary: Allow webkitEnterFullScreen from outside user gesture when restrictions are u...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
: 63202 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-02-25 11:11 PST by Dean Jackson
Modified: 2011-06-23 03:45 PDT (History)
2 users (show)

See Also:


Attachments
Patch (2.49 KB, patch)
2011-02-25 11:25 PST, Dean Jackson
eric.carlson: review-
Details | Formatted Diff | Diff
Updated patch (4.46 KB, patch)
2011-02-25 12:30 PST, Dean Jackson
eric.carlson: review-
Details | Formatted Diff | Diff
Another update (4.48 KB, patch)
2011-02-25 13:23 PST, Dean Jackson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.