Bug 137472 – [Media] Expose AudioTracks in the "captions" menu.

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

[Media] Expose AudioTracks in the "captions" menu.

[Jer Noble]

Created attachment 239370 [details]
Patch

[Jer Noble]

Created attachment 239373 [details]
Patch

[Build Bot]

Comment on attachment 239373 [details]
Patch

Attachment 239373 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5349634390097920

New failing tests:
media/video-controls-rendering.html
media/controls-strict.html
media/video-volume-slider.html
media/video-display-toggle.html
media/audio-controls-rendering.html
editing/unsupported-content/list-delete-003.html
editing/unsupported-content/table-type-before.html
editing/unsupported-content/list-delete-001.html
editing/unsupported-content/list-type-after.html
accessibility/media-element.html
media/controls-without-preload.html
editing/unsupported-content/table-delete-002.html
editing/unsupported-content/list-type-before.html
editing/unsupported-content/table-type-after.html

[Build Bot]

Created attachment 239377 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

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

[Build Bot]

Comment on attachment 239373 [details]
Patch

Attachment 239373 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6299740211773440

New failing tests:
media/controls-strict.html
media/video-volume-slider.html
media/video-display-toggle.html
media/audio-controls-rendering.html
media/video-controls-rendering.html
accessibility/media-element.html
media/controls-without-preload.html
media/controls-after-reload.html

[Build Bot]

Created attachment 239378 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

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

[Jer Noble]

Created attachment 239380 [details]
Patch

[Brent Fulgham]

Comment on attachment 239380 [details]
Patch

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

I'm worried that we need to flesh out the AudioTrack stuff on Windows; there isn't any such thing as an AudioTrackPrivateAVCF/VideoTrackPrivateAVCF. Otherwise, this looks great! Let's get it landed and if anything breaks on Windows I'll clean it up during my ongoing media work.

> Source/WebCore/html/track/AudioTrack.cpp:-150
> -    setEnabled(enabled);

Just Curious: Why don't we want to recurse into the private 'setEnabled' here? What problem does that fix?

> Source/WebCore/page/CaptionUserPreferences.cpp:228
> +    });

Oooh!  A lambda! :-)

[Jer Noble]

(In reply to comment #8)
> (From update of attachment 239380 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239380&action=review
> 
> I'm worried that we need to flesh out the AudioTrack stuff on Windows; there isn't any such thing as an AudioTrackPrivateAVCF/VideoTrackPrivateAVCF. Otherwise, this looks great! Let's get it landed and if anything breaks on Windows I'll clean it up during my ongoing media work.

Thanks!

> > Source/WebCore/html/track/AudioTrack.cpp:-150
> > -    setEnabled(enabled);
> 
> Just Curious: Why don't we want to recurse into the private 'setEnabled' here? What problem does that fix?

Previously, calling AudioTrack::setEnabled() calls
  AudioTrackPrivate::setEnabled() which calls
    AudioTrack::enabledChanged() which calls
      AudioTrackPrivate::setEnabled() which ...

The chain only breaks when the AudioTrackPrivate notices that the enabled param == the stored value of enabled.  What if some future AudioTrackPrivate doesn't cache the value of enabled? Or doesn't check?

> > Source/WebCore/page/CaptionUserPreferences.cpp:228
> > +    });
> 
> Oooh!  A lambda! :-)

[WebKit Commit Bot]

Comment on attachment 239380 [details]
Patch

Clearing flags on attachment: 239380

Committed r174402: <http://trac.webkit.org/changeset/174402>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

This broke a test:
https://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK2%20(Tests)/r174402%20(7248)/results.html

[Alexey Proskuryakov]

https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Ftrack%2Ftrack-forced-subtitles-in-band.html

-EXPECTED (trackMenuItems.length == '6') OK
+EXPECTED (trackMenuItems.length == '6'), OBSERVED '3' FAIL

[Jer Noble]

(In reply to comment #13)
> https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Ftrack%2Ftrack-forced-subtitles-in-band.html
> 
> -EXPECTED (trackMenuItems.length == '6') OK
> +EXPECTED (trackMenuItems.length == '6'), OBSERVED '3' FAIL

https://bugs.webkit.org/show_bug.cgi?id=137505

[Jon Lee]

rdar://problem/13466748