Bug 83095 - apple.com top navigation bar appears inside video during full screen exit animation
Summary: apple.com top navigation bar appears inside video during full screen exit ani...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL: http://www.apple.com/education/#video...
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-04-03 16:55 PDT by Jer Noble
Modified: 2012-04-20 15:34 PDT (History)
5 users (show)

See Also:


Attachments
Patch (22.58 KB, patch)
2012-04-04 13:13 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (22.57 KB, patch)
2012-04-04 13:23 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (22.53 KB, patch)
2012-04-04 13:52 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (25.15 KB, patch)
2012-04-06 16:14 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (27.76 KB, patch)
2012-04-06 16:22 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (29.16 KB, patch)
2012-04-06 18:13 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (29.65 KB, patch)
2012-04-09 09:57 PDT, Jer Noble
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2012-04-03 16:55:51 PDT
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.
Comment 1 Jer Noble 2012-04-03 16:56:06 PDT
<rdar://problem/11106087>
Comment 2 Jer Noble 2012-04-04 13:13:27 PDT
Created attachment 135657 [details]
Patch
Comment 3 Jer Noble 2012-04-04 13:23:21 PDT
Created attachment 135661 [details]
Patch

Rebaselined patch against ToT.
Comment 4 Jer Noble 2012-04-04 13:52:34 PDT
Created attachment 135669 [details]
Patch

Rebaselined again; this time it'll stick.
Comment 5 Build Bot 2012-04-04 14:24:13 PDT
Comment on attachment 135669 [details]
Patch

Attachment 135669 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12329072
Comment 6 WebKit Review Bot 2012-04-04 14:41:17 PDT
Comment on attachment 135669 [details]
Patch

Attachment 135669 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12328096
Comment 7 Jer Noble 2012-04-06 16:14:22 PDT
Created attachment 136088 [details]
Patch
Comment 8 Jer Noble 2012-04-06 16:22:32 PDT
Created attachment 136092 [details]
Patch

Added hasCustomFullScreenBehavior accessor to the Chromium version of LayoutTestController.
Comment 9 Gustavo Noronha (kov) 2012-04-06 17:48:16 PDT
Comment on attachment 136092 [details]
Patch

Attachment 136092 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12360155
Comment 10 Jer Noble 2012-04-06 18:13:34 PDT
Created attachment 136113 [details]
Patch

Fix the GTK build by re-exporting the necessary symbols.
Comment 11 Gustavo Noronha (kov) 2012-04-06 19:14:24 PDT
Comment on attachment 136113 [details]
Patch

Attachment 136113 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12363183
Comment 12 WebKit Review Bot 2012-04-06 22:47:48 PDT
Comment on attachment 136113 [details]
Patch

Attachment 136113 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12363263
Comment 13 Jer Noble 2012-04-09 09:57:09 PDT
Created attachment 136245 [details]
Patch

Fixed up the GTK and Chromium port parts of the patch.
Comment 14 Eric Carlson 2012-04-19 09:58:01 PDT
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.
Comment 15 Jer Noble 2012-04-20 15:34:30 PDT
Committed r114790: <http://trac.webkit.org/changeset/114790>