Bug 157975 – REGRESSION (r200196): Default audio controls’ volume slider is clipped out

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

To reproduce, navigate to data:text/html,<audio%20controls> and hover over the volume icon. Notice that the volume slider doesn’t show. This is happening because the -webkit-media-controls pseudoelement has overflow: hidden, causing the slider to get clipped out.

<rdar://problem/26408506>

Caused by <http://trac.webkit.org/r200196>, the fix for bug 157110.

[Ada Chan]

Also <rdar://problem/26186246>

[Ada Chan]

Created attachment 279627 [details]
Patch

[Ada Chan]

Submitted initial patch to run through EWS. Pretty sure I need to rebaseline some test results.

[Build Bot]

Comment on attachment 279627 [details]
Patch

Attachment 279627 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1373045

New failing tests:
media/media-document-audio-repaint.html
media/video-zoom-controls.html

[Build Bot]

Created attachment 279629 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 279627 [details]
Patch

Attachment 279627 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1373096

New failing tests:
media/media-document-audio-repaint.html
media/video-zoom-controls.html

[Build Bot]

Created attachment 279630 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 279627 [details]
Patch

Attachment 279627 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1373102

New failing tests:
media/media-document-audio-repaint.html
media/video-zoom-controls.html

[Build Bot]

Created attachment 279632 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Ada Chan]

Created attachment 279634 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 279634 [details]
Patch

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

> LayoutTests/TestExpectations:57
> +# Only test the following on Mac due to its different styling on the media controls container
> +media/controls-panel-not-clipped-out.html [ Skip ]
> +

Why don't we put this under platform/mac/media instead?

[Ada Chan]

(In reply to comment #13)
> Comment on attachment 279634 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=279634&action=review
> 
> > LayoutTests/TestExpectations:57
> > +# Only test the following on Mac due to its different styling on the media controls container
> > +media/controls-panel-not-clipped-out.html [ Skip ]
> > +
> 
> Why don't we put this under platform/mac/media instead?

I talked to Alexey more about this, and he suggested adding a media/mac folder and putting the test there.  That folder will be skipped on other platforms.

Updated patch coming soon.

[Ada Chan]

Created attachment 279671 [details]
Patch

[Ryosuke Niwa]

(In reply to comment #14)
> (In reply to comment #13)
> > Comment on attachment 279634 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=279634&action=review
> > 
> > > LayoutTests/TestExpectations:57
> > > +# Only test the following on Mac due to its different styling on the media controls container
> > > +media/controls-panel-not-clipped-out.html [ Skip ]
> > > +
> > 
> > Why don't we put this under platform/mac/media instead?
> 
> I talked to Alexey more about this, and he suggested adding a media/mac
> folder and putting the test there.  That folder will be skipped on other
> platforms.

But LayoutTests/platform/mac/media is such a folder already though.  And run-webkit-tests knows to run tests inside platform/mac/media when asked to run LayoutTests/media/

[Ada Chan]

(In reply to comment #16)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > Comment on attachment 279634 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=279634&action=review
> > > 
> > > > LayoutTests/TestExpectations:57
> > > > +# Only test the following on Mac due to its different styling on the media controls container
> > > > +media/controls-panel-not-clipped-out.html [ Skip ]
> > > > +
> > > 
> > > Why don't we put this under platform/mac/media instead?
> > 
> > I talked to Alexey more about this, and he suggested adding a media/mac
> > folder and putting the test there.  That folder will be skipped on other
> > platforms.
> 
> But LayoutTests/platform/mac/media is such a folder already though.  And
> run-webkit-tests knows to run tests inside platform/mac/media when asked to
> run LayoutTests/media/

Alexey's argument is that it's a little weird to have tests in platform, and that it's better to keep platform-specific results in platform.

[Ada Chan]

Committed:
http://trac.webkit.org/changeset/201355

[Alexey Proskuryakov]

We have already went through some effort moving tests out of platform and into better named directories in normal locations (ideally, named after a specific technology, but otherwise, just named under the platform).

It gets really weird when platform specific tests have platform specific results. E.g., what is the correct location for results when Yosemite doesn't match El Capitan, and WK1 doesn't match WK2, and the test is in platform/mac/media? Is it platform/mac-yosemite/wk2/media, platform/mac-yosemite/wk2/platform/mac/media, or something else?