Description: http://code.google.com/p/chromium/issues/detail?id=155099 The issue within the code is that updateActiveTextTrackCues is called when one track has mode = "showing" and the other one has mode = "hidden". This means that cues from both tracks get added in the cue tree, but only the ones from the first track get rendered. When the second track gets the mode set to "showing", its cues were already added in the cue tree and marked as active, and therefore activeSetChanged will be false and not rendering will occur.
Created attachment 182875 [details] Simple fix and test case
FWIW: LGTM.
Comment on attachment 182875 [details] Simple fix and test case Attachment 182875 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15904203 New failing tests: media/video-controls-captions.html
Created attachment 182934 [details] Updated test result The video-controls-captions test still works as expected, it just outputs a different error
Comment on attachment 182934 [details] Updated test result View in context: https://bugs.webkit.org/attachment.cgi?id=182934&action=review I really like the new test - it is concise, thorough, and easy to understand. Good fix, thanks! > LayoutTests/media/track/track-cue-rendering-mode-changed.html:12 > + var testTrackEnglish, testTrackArabic; Nit: each variable should be declared independently. > LayoutTests/media/track/track-cue-rendering-mode-changed.html:15 > + function addTracks() { Nit: a function's opening brace should be on a new line. > LayoutTests/media/track/track-cue-rendering-mode-changed.html:16 > + findMediaElement(); Nit: this should not be necessary, it was call in loaded().
Created attachment 183038 [details] Patch for landing
Comment on attachment 183038 [details] Patch for landing Clearing flags on attachment: 183038 Committed r139932: <http://trac.webkit.org/changeset/139932>
All reviewed patches have been landed. Closing bug.
(In reply to comment #5) Fixed the comments in the landing patch. Thanks for your fast review, Eric!
media/video-transformed.html started crashing on Mac shortly after this landed. Any idea if that is related?
(In reply to comment #10) > media/video-transformed.html started crashing on Mac shortly after this landed. > Any idea if that is related? From the backtrace, eg. http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r139933%20(4912)/media/video-transformed-crash-log.txt, I don't think so.
Comment on attachment 183038 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=183038&action=review > Source/WebCore/html/HTMLMediaElement.cpp:1175 > + if (!activeSetChanged) { > + // Even though the active set has not changed, it is possible that the > + // the mode of a track has changed from 'hidden' to 'showing' and the > + // cues have not yet been rendered. > + if (hasMediaControls()) > + mediaControls()->updateTextTrackDisplay(); > + This sometimes causes an assert in MediaControlTextTrackContainerElement::updateDisplay after a track is removed, because m_currentlyActiveCues has not been updated.
(In reply to comment #12) > (From update of attachment 183038 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=183038&action=review > > > Source/WebCore/html/HTMLMediaElement.cpp:1175 > > + if (!activeSetChanged) { > > + // Even though the active set has not changed, it is possible that the > > + // the mode of a track has changed from 'hidden' to 'showing' and the > > + // cues have not yet been rendered. > > + if (hasMediaControls()) > > + mediaControls()->updateTextTrackDisplay(); > > + > > This sometimes causes an assert in MediaControlTextTrackContainerElement::updateDisplay after a track is removed, because m_currentlyActiveCues has not been updated. Indeed; since it's a new issue, filed a bug to track it: https://bugs.webkit.org/show_bug.cgi?id=107465