Closed Bug 1275808 Opened 4 years ago Closed 4 years ago
[webvtt] Merge the Text
Track Manager::Time Marches On() and Update Cue Display() function
MozReview Request: Bug 1275808 - Move the UpdateCueDisplay() into TimeMarchesOn() as step 18. r=rillian
58 bytes, text/x-review-board-request
Since the TimeMarchesOn() is called at the end of UpdateCueDisplay(), we should merge them into a function. Also the DispatchTimeMarchesOn() should dispatch a task that runs UpdateCueDisplay()+TimeMarchesOn(). In addition, since we have precise time of every cues, we can invoke the new function more precisely instead every timeupdate, so that we may improve the performance without drawing the same cue at the dom tree through WebVTTParserWrapper.
4 years ago
Priority: -- → P2
Review commit: https://reviewboard.mozilla.org/r/56500/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56500/
Attachment #8758131 - Flags: review?(giles)
https://reviewboard.mozilla.org/r/56498/#review53060 Invoke the UpdateCueDisplay() at the end of TimeMarchesOn() as step 18, and we expose the TimeMarchesOn() and DispatchTimeMarchesOn() as sync/async interface for usage. The sync function TimeMarchesOn() will fix some bugs if we call it at the right time, ex: change subtitle when MediaElement is paused, the cue will be shown immediately if we call TimeMarchesOn(). And the async function DispatchTimeMarchesOn() is called when the scritp add/remove texttrack/texttrackcue. And another benefit of calling UpdateCueDisplay() in TimeMarchesOn() is that there are some early return during TimeMarchesOn() so that we won't redraw the same cue at the dom tree.
Comment on attachment 8758131 [details] MozReview Request: Bug 1275808 - Move the UpdateCueDisplay() into TimeMarchesOn() as step 18. r=rillian https://reviewboard.mozilla.org/r/56500/#review53374 I didn't see an obvious cause for the windows debug mda crashes. Otherwise looks good. ::: dom/html/TextTrackManager.h:105 (Diff revision 1) > > private: > - void TimeMarchesOn(); > + /** > + * Converts the TextTrackCue's cuetext into a tree of DOM objects and attaches > + * it to a div on it's owning TrackElement's MediaElement's caption overlay. > + */ s/it's/its/ while we're here.
Attachment #8758131 - Flags: review?(giles) → review+
Comment on attachment 8758131 [details] MozReview Request: Bug 1275808 - Move the UpdateCueDisplay() into TimeMarchesOn() as step 18. r=rillian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56500/diff/1-2/
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/e059a7af074e Move the UpdateCueDisplay() into TimeMarchesOn() as step 18. r=rillian
You need to log in before you can comment on or make changes to this bug.