Bug 83095 – apple.com top navigation bar appears inside video during full screen exit animation

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jer Noble]

When exiting full-screen playback of the video at <http://www.apple.com/education/#video-textbooks>, the shrinking video has the site’s top navigation bar superimposed on it.

[Jer Noble]

<rdar://problem/11106087>

[Jer Noble]

Created attachment 135657 [details]
Patch

[Jer Noble]

Created attachment 135661 [details]
Patch

Rebaselined patch against ToT.

[Jer Noble]

Created attachment 135669 [details]
Patch

Rebaselined again; this time it'll stick.

[Build Bot]

Comment on attachment 135669 [details]
Patch

Attachment 135669 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12329072

[WebKit Review Bot]

Comment on attachment 135669 [details]
Patch

Attachment 135669 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12328096

[Jer Noble]

Created attachment 136088 [details]
Patch

[Jer Noble]

Created attachment 136092 [details]
Patch

Added hasCustomFullScreenBehavior accessor to the Chromium version of LayoutTestController.

[Gustavo Noronha (kov)]

Comment on attachment 136092 [details]
Patch

Attachment 136092 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12360155

[Jer Noble]

Created attachment 136113 [details]
Patch

Fix the GTK build by re-exporting the necessary symbols.

[Gustavo Noronha (kov)]

Comment on attachment 136113 [details]
Patch

Attachment 136113 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12363183

[WebKit Review Bot]

Comment on attachment 136113 [details]
Patch

Attachment 136113 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12363263

[Jer Noble]

Created attachment 136245 [details]
Patch

Fixed up the GTK and Chromium port parts of the patch.

[Eric Carlson]

Comment on attachment 136245 [details]
Patch

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

> Source/WebCore/ChangeLog:22
> +        * dom/Document.cpp:
> +        (WebCore::Document::webkitWillExitFullScreenForElement):
> +        (WebCore::Document::webkitDidExitFullScreenForElement):
> +
> +        To facilitate writing reproducible LayoutTests, expose webkitWill/Did/Enter/ExitFullScreen
> +        from the Internals object, so scripts can call them explicitly:
> +        * testing/Internals.cpp:
> +        (WebCore::Internals::webkitWillEnterFullScreenForElement):
> +        (WebCore::Internals::webkitDidEnterFullScreenForElement):
> +        (WebCore::Internals::webkitWillExitFullScreenForElement):
> +        (WebCore::Internals::webkitDidExitFullScreenForElement):

Nit: having comments about what changed in each function would have made this much easier for someone that doesn't know so much about fullscreen (me) review this.

> Source/WebCore/dom/Document.cpp:-5448
> -    if (!attached() || inPageCache())
> -        return;

Nit: please add a comment about why this is no longer necessary to the ChangLog.

> Source/WebCore/dom/Document.cpp:-5457
> -    if (!attached() || inPageCache())

Ditto.

> LayoutTests/fullscreen/full-screen-exit-animation-stacking-context.html:7
> +            function init() {

Nit: Function opening brace on its own line please. Here and throughout the file.

[Jer Noble]

Committed r114790: <http://trac.webkit.org/changeset/114790>