[GTK] Volume slider shows below the panel with videos in certain cases
Created attachment 227458 [details] Patch
Comment on attachment 227458 [details] Patch Can you explain why this was failing before?
Created attachment 227463 [details] Patch
(In reply to comment #2) > (From update of attachment 227458 [details]) > Can you explain why this was failing before? Sure, I forgot to explain it in the changelog, so reuploaded the patch.
Comment on attachment 227463 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227463&action=review Looks good. A couple small nits for landing: > Source/WebCore/ChangeLog:9 > + We need to delay the moment we check if the volume slider show > + show up or down because if the video was not visible when we were Double "show" > LayoutTests/media/video-initially-hidden-volume-slider-up-expected.txt:5 > +** Test that the volume slider is rendered correctly if video is brought to foreground ** > +** Move mouse on top of the mute button ** > +** Ensure layout is done after mouse move ** > +** The volume slider should not be positioned below the panel ** I think these can just be code comments.
Comment on attachment 227463 [details] Patch Attachment 227463 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6752269651410944 New failing tests: media/video-initially-hidden-volume-slider-up.html
Created attachment 227479 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 227463 [details] Patch Attachment 227463 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4879779337601024 New failing tests: media/video-initially-hidden-volume-slider-up.html
Created attachment 227482 [details] Archive of layout-test-results from webkit-ews-07 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 227463 [details] Patch Attachment 227463 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5351491837624320 New failing tests: media/video-initially-hidden-volume-slider-up.html
Created attachment 227486 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 227546 [details] Patch
(In reply to comment #5) > > Source/WebCore/ChangeLog:9 > > + We need to delay the moment we check if the volume slider show > > + show up or down because if the video was not visible when we were > > Double "show" I rephrased it. > > LayoutTests/media/video-initially-hidden-volume-slider-up-expected.txt:5 > > +** Test that the volume slider is rendered correctly if video is brought to foreground ** > > +** Move mouse on top of the mute button ** > > +** Ensure layout is done after mouse move ** > > +** The volume slider should not be positioned below the panel ** > > I think these can just be code comments. Done. I also fixed the problem with the test in Mac.
(In reply to comment #13) > (In reply to comment #5) > > > Source/WebCore/ChangeLog:9 > > > + We need to delay the moment we check if the volume slider show > > > + show up or down because if the video was not visible when we were > > > > Double "show" > > I rephrased it. > > > > LayoutTests/media/video-initially-hidden-volume-slider-up-expected.txt:5 > > > +** Test that the volume slider is rendered correctly if video is brought to foreground ** > > > +** Move mouse on top of the mute button ** > > > +** Ensure layout is done after mouse move ** > > > +** The volume slider should not be positioned below the panel ** > > > > I think these can just be code comments. > > Done. > > I also fixed the problem with the test in Mac. Apart from this changes, the fix for Mac 'required' to change some common Mac code, I think a review for that would be necessary as I think the other r+ is not enough for this.
Comment on attachment 227546 [details] Patch r=me, based on the Mac changes & Martin's earlier r+ and
(In reply to comment #15) > (From update of attachment 227546 [details]) > r=me, based on the Mac changes & Martin's earlier r+ and s/ and/.
Comment on attachment 227546 [details] Patch Clearing flags on attachment: 227546 Committed r166227: <http://trac.webkit.org/changeset/166227>
All reviewed patches have been landed. Closing bug.