Bug 101100 – [BlackBerry] Automatically go fullscreen on video play

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Max Feil]

There is a requirement to have HTML5 video automatically enter fullscreen when the video starts playing. This patch implements this feature, with restrictions. The main restriction is adherence to WebKit's philosophy of only entering fullscreen due to a user gesture. This is important in order to avoid pop-up advertisements and other unwanted fullscreen content. One consequence of this is that video elements with the autoplay attribute will not automatically enter fullscreen.

Other caveats:
- This feature applies only to "small screen" devices where automatically going fullscreen makes more sense.
- Fullscreen will only be entered automatically when the video is played from the beginning (current time is zero). It is assumed that if the user is resuming play from a paused state and is not in fullscreen mode, then they exited fullscreen mode intentionally.

[Max Feil]

Created attachment 172148 [details]
Patch

[Antonio Gomes]

Comment on attachment 172148 [details]
Patch

It looks good to me. I will let Eric to have a change to look at the cross platform additions

[Eric Carlson]

Comment on attachment 172148 [details]
Patch

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

Marking r+, but I would prefer the name change.

> Source/WebCore/html/HTMLMediaElement.cpp:4544
> +bool HTMLMediaElement::mediaPlayerIsFullscreenRestricted() const
> +{
> +    return userGestureRequiredForFullscreen() && !ScriptController::processingUserGesture();
> +}

I think it would be better to flip the meaning of the name so you ask if fullscreen is allowed instead of restricted. Maybe "mediaPlayerIsFullscreenAllowed", or "mediaPlayerIsFullscreenPermitted"?

[Max Feil]

Created attachment 172385 [details]
Patch

[Max Feil]

Comment on attachment 172385 [details]
Patch

Marking reviewed based on r+ given to previous patch. The only change in this patch is a simple one to rename boolean function mediaPlayerIsFullscreenRestricted() to mediaPlayerIsFullscreenPermitted() and change its sense.

[Rob Buis]

Comment on attachment 172385 [details]
Patch

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

> Source/WebCore/ChangeLog:38
> +        (WebCore::MediaPlayerClient::mediaPlayerIsFullscreenRestricted):

Hate to be pedantic, but this does not match the renaming.

[Max Feil]

Created attachment 172442 [details]
Patch

Good eye, Rob. I forgot about the change log.

[WebKit Review Bot]

Comment on attachment 172442 [details]
Patch

Clearing flags on attachment: 172442

Committed r133606: <http://trac.webkit.org/changeset/133606>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.