Extend the rules for updating the display of WebVTT text tracks.
no spec, no extension; propose something in the W3C or WHATWG CG, get it written into a spec, then reopen this bug
As noted in the master bug, there is a proposed solution [1] [2] which has had a great deal of discussion in the text community group mailing list (eg. [3]). Please don't close bugs without discussion. [1] https://dvcs.w3.org/hg/text-tracks/raw-file/default/608toVTT/region.html [2] http://www.w3.org/community/texttracks/wiki/MultiCueBox [3] http://lists.w3.org/Archives/Public/public-texttracks/2012Dec/0000.html
This work was landed in Chromium under commit 0cdebc1d76d41bfcb9c4b022e54a4fbff132225d, and should be merged.
Created attachment 226603 [details] WIP Patch Not Tested Yet
Created attachment 226754 [details] Patch
Comment on attachment 226754 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226754&action=review > Source/WebCore/html/shadow/MediaControlElements.cpp:1301 > + TextTrackRegion* region = cue->track()->regions()->getRegionById(regionId); Both TextTrackCue::track and TextTrack::regions() can return NULL, either of which will make this crash. > Source/WebCore/html/track/TextTrackRegion.cpp:334 > + DEPRECATED_DEFINE_STATIC_LOCAL(const AtomicString, trackRegionCueContainerScrollingClass, ("scrolling", AtomicString::ConstructFromLiteral)); Nit: you might as well go straight to NeverDestroyed<T>. > Source/WebCore/html/track/TextTrackRegion.cpp:341 > + DEPRECATED_DEFINE_STATIC_LOCAL(const AtomicString, trackRegionCueContainerPseudoId, Ditto. > Source/WebCore/html/track/TextTrackRegion.cpp:349 > + DEPRECATED_DEFINE_STATIC_LOCAL(const AtomicString, trackRegionShadowPseudoId, Ditto. > Source/WebCore/html/track/TextTrackRegion.cpp:366 > + LOG(Media, "TextTrackRegion::displayLastTextTrackCueBox"); Nit: won't this will be call really frequently? if so, you might not want to log here. > Source/WebCore/html/track/VTTCue.cpp:223 > + displayTreeInternal()->remove(ASSERT_NO_EXCEPTION); displayTreeInternal allocates the display tree if it is NULL, so you should return "if (!hasDisplayTree())" > Source/WebCore/html/track/VTTCue.cpp:747 > + TextTrackRegion* region = track()->regions()->getRegionById(m_regionId); TextTrackCue::track() and TextTrack::regions() can both return NULL
Comment on attachment 226754 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226754&action=review >> Source/WebCore/html/track/TextTrackRegion.cpp:334 >> + DEPRECATED_DEFINE_STATIC_LOCAL(const AtomicString, trackRegionCueContainerScrollingClass, ("scrolling", AtomicString::ConstructFromLiteral)); > > Nit: you might as well go straight to NeverDestroyed<T>. Or maybe not, see https://bugs.webkit.org/show_bug.cgi?id=130256#c3.
Comment on attachment 226754 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226754&action=review >> Source/WebCore/html/shadow/MediaControlElements.cpp:1301 >> + TextTrackRegion* region = cue->track()->regions()->getRegionById(regionId); > > Both TextTrackCue::track and TextTrack::regions() can return NULL, either of which will make this crash. Yikes! cue->track() is tested a few lines above, but I'll add code for the regions() == nullptr case.
Committed r165765: <http://trac.webkit.org/changeset/165765>
*** Bug 117469 has been marked as a duplicate of this bug. ***