Bug 84378

Summary: [GTK] media/track/track-cue-rendering-snap-to-lines-not-set.html fails
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: WebKitGTKAssignee: Philippe Normand <pnormand>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, feature-media-reviews, macpherson, menard, vcarbune, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch eric.carlson: review+, buildbot: commit-queue-

Description Philippe Normand 2012-04-19 13:32:14 PDT
Consistently. Diff:

--- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/media/track/track-cue-rendering-snap-to-lines-not-set-expected.txt 
+++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/media/track/track-cue-rendering-snap-to-lines-not-set-actual.txt 
@@ -2,7 +2,7 @@
 EVENT(canplaythrough)
 
 The track container and the media controls should completely cover the video element
-EXPECTED (cueDisplayContainer.offsetHeight + controlsPanelElement.offsetHeight == video.videoHeight == 'true') OK
+EXPECTED (cueDisplayContainer.offsetHeight + controlsPanelElement.offsetHeight == video.videoHeight == 'true'), OBSERVED 'false' FAIL
 EXPECTED (cueDisplayContainer.offsetWidth == video.videoWidth == 'true') OK
 
 ** Default **

This test was introduced in 
http://trac.webkit.org/changeset/114640
Comment 1 Victor Carbune 2012-04-19 13:50:11 PDT
(In reply to comment #0)
> Consistently. Diff:
> 
> --- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/media/track/track-cue-rendering-snap-to-lines-not-set-expected.txt 
> +++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/media/track/track-cue-rendering-snap-to-lines-not-set-actual.txt 
> @@ -2,7 +2,7 @@
>  EVENT(canplaythrough)
> 
>  The track container and the media controls should completely cover the video element
> -EXPECTED (cueDisplayContainer.offsetHeight + controlsPanelElement.offsetHeight == video.videoHeight == 'true') OK
> +EXPECTED (cueDisplayContainer.offsetHeight + controlsPanelElement.offsetHeight == video.videoHeight == 'true'), OBSERVED 'false' FAIL
>  EXPECTED (cueDisplayContainer.offsetWidth == video.videoWidth == 'true') OK
> 
>  ** Default **
> 
> This test was introduced in 
> http://trac.webkit.org/changeset/114640
I believe this particular check fails because the -webkit-media-controls-panel element is absolutely positioned in mediaControlsGTK.

This change from absolute to relative positioning is required because the area on top of the video is now a flexible box, with two main boxes on top of each other: cue container box and control panel element.

If the panel element is absolutely positioned the flexible box is not positioning it, and the cue container expands towards the bottom of the video.
Comment 2 Philippe Normand 2012-04-20 16:10:06 PDT
(In reply to comment #1)
> (In reply to comment #0)
> > Consistently. Diff:
> > 
> > --- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/media/track/track-cue-rendering-snap-to-lines-not-set-expected.txt 
> > +++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/media/track/track-cue-rendering-snap-to-lines-not-set-actual.txt 
> > @@ -2,7 +2,7 @@
> >  EVENT(canplaythrough)
> > 
> >  The track container and the media controls should completely cover the video element
> > -EXPECTED (cueDisplayContainer.offsetHeight + controlsPanelElement.offsetHeight == video.videoHeight == 'true') OK
> > +EXPECTED (cueDisplayContainer.offsetHeight + controlsPanelElement.offsetHeight == video.videoHeight == 'true'), OBSERVED 'false' FAIL
> >  EXPECTED (cueDisplayContainer.offsetWidth == video.videoWidth == 'true') OK
> > 
> >  ** Default **
> > 
> > This test was introduced in 
> > http://trac.webkit.org/changeset/114640
> I believe this particular check fails because the -webkit-media-controls-panel element is absolutely positioned in mediaControlsGTK.
> 
> This change from absolute to relative positioning is required because the area on top of the video is now a flexible box, with two main boxes on top of each other: cue container box and control panel element.
> 
> If the panel element is absolutely positioned the flexible box is not positioning it, and the cue container expands towards the bottom of the video.

Thanks Victor!
I was just trying to figure out why the media controls panel was invisible now... And I remembered this bug. Will try your proposed solution and upload a patch.
Comment 3 Philippe Normand 2012-04-20 16:19:46 PDT
The invisible controls timeline is a different issue, sorry for the noise.
Comment 4 Philippe Normand 2012-05-01 09:18:37 PDT
Created attachment 139635 [details]
Patch
Comment 5 Philippe Normand 2012-05-01 09:20:00 PDT
(In reply to comment #4)
> Created an attachment (id=139635) [details]
> Patch

When landing this the files DerivedSources/WebCore/UserAgentStyleSheetsData* should be removed on the bots to ensure the updated CSS file is parsed during the build.
Comment 6 Build Bot 2012-05-01 09:40:09 PDT
Comment on attachment 139635 [details]
Patch

Attachment 139635 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12587591
Comment 7 Philippe Normand 2012-05-02 07:39:45 PDT
Committed r115829: <http://trac.webkit.org/changeset/115829>
Comment 8 Philippe Normand 2012-05-02 08:14:10 PDT
Hum this patch introduced a regression: bug 85376