Hide the CC button for <audio> with <track>s

Hide the CC button for <audio> with <track>s

It is per spec that the DOM API should work the same for <audio>
and <video>, but it doesn't make any sense to have the UI bits for
<audio> since there's nowhere to actually render the cues.

BUG=314027
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161246

[philipj_slow, 2013-11-01 11:58:38]

Adam and Adam, you've each reviewed one of my previous patches here, so here's
more for you.

Somewhat surprisingly, nothing in LayoutTests/media/ started failing with this
change, so what's the proper way to test this? A pixel test I presume, but how
to generate the baselines for platforms I'm not on?

[vcarbune.chromium, 2013-11-01 12:15:47]

Not my review, but chiming in a bit, hope you don't mind.

On 2013/11/01 11:58:38, philipj wrote:
> Somewhat surprisingly, nothing in LayoutTests/media/ started failing with this
> change, so what's the proper way to test this? A pixel test I presume, but how
> to generate the baselines for platforms I'm not on?

I don't think there's something that could have failed as I don't think there's
any test having a <track> element as a child of <audio>

We have generally avoided pixel tests for controls and captions. Pixel tests
make sense to check that each control is actually rendered as expected when it
is displayed, but just a check whether it appears or not, can be done with
testClosedCaptionsButtonVisibility from media-controls.js.

Just pick an existing test and simplify it to have an audio element with a text
track as child
https://code.google.com/p/chromium/codesearch#search/&q=testClosedCaptionsBut...

[philipj_slow, 2013-11-01 14:37:54]

On 2013/11/01 12:15:47, vcarbune.chromium wrote:
> Not my review, but chiming in a bit, hope you don't mind.
> 
> On 2013/11/01 11:58:38, philipj wrote:
> > Somewhat surprisingly, nothing in LayoutTests/media/ started failing with
this
> > change, so what's the proper way to test this? A pixel test I presume, but
how
> > to generate the baselines for platforms I'm not on?
> 
> I don't think there's something that could have failed as I don't think
there's
> any test having a <track> element as a child of <audio>
> 
> We have generally avoided pixel tests for controls and captions. Pixel tests
> make sense to check that each control is actually rendered as expected when it
> is displayed, but just a check whether it appears or not, can be done with
> testClosedCaptionsButtonVisibility from media-controls.js.
> 
> Just pick an existing test and simplify it to have an audio element with a
text
> track as child
>
https://code.google.com/p/chromium/codesearch#search/&q=testClosedCaptionsBut...

Ah, we have internals.shadowRoot to enable testing of this, excellent!

[abarth-chromium, 2013-11-01 19:38:49]

I'm not the right person to review this CL.  Maybe adamk is?

[philipj_slow, 2013-11-01 21:50:15]

There, I added a simple test, which I've verified failed before the CSS changes.

[nessy, 2013-11-02 02:14:00]

FWIW: LGTM - glad somebody is picking up the slack around video controls!

[Mike West, 2013-11-04 10:07:11]

On 2013/11/02 02:14:00, nessy wrote:
> FWIW: LGTM - glad somebody is picking up the slack around video controls!

If Silvia is happy, I'm happy. LGTM.

I can certainly imagine someone wanting to provide captions for audio, however.
The WHATWG spec has a non-normative note which reads "If captions or a sign
language video are available, the video element can be used instead of the audio
element to play the audio, allowing users to enable the visual alternatives".
That sounds strange; why should an author move to a video element if there's no
video?

I'm fine with removing this for the moment since it's nonfunctional, but it
seems reasonable to support the use case in the future via some mechanism.

[philipj_slow, 2013-11-04 10:29:49]

On 2013/11/04 10:07:11, Mike West wrote:
> On 2013/11/02 02:14:00, nessy wrote:
> > FWIW: LGTM - glad somebody is picking up the slack around video controls!
> 
> If Silvia is happy, I'm happy. LGTM.
> 
> I can certainly imagine someone wanting to provide captions for audio,
however.
> The WHATWG spec has a non-normative note which reads "If captions or a sign
> language video are available, the video element can be used instead of the
audio
> element to play the audio, allowing users to enable the visual alternatives".
> That sounds strange; why should an author move to a video element if there's
no
> video?
> 
> I'm fine with removing this for the moment since it's nonfunctional, but it
> seems reasonable to support the use case in the future via some mechanism.

There's no technical reason that the spec couldn't allow rendering of subtitles
in an audio element, but there just doesn't seem to be a compelling reason to
make it so, given that the poster image or video isn't rendered for <audio>.
I've also tested, and Chromium currently doesn't render subtitles in <audio>, so
removing the useless controls is good.

[commit-bot: I haz the power, 2013-11-04 10:30:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/54713007/80001

[commit-bot: I haz the power, 2013-11-04 11:25:33]

Change committed as 161246