[webvtt] Implement the TextTrackCue::display state

RESOLVED FIXED in mozilla25

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: reyre, Assigned: reyre)

Tracking

Trunk
mozilla25
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
This is used in the rendering model to keep the display of cues consistent.
(Assignee)

Updated

5 years ago
Blocks: 882700
(Assignee)

Updated

5 years ago
Blocks: 865407
(Assignee)

Comment 1

5 years ago
As far as I can tell this is basically the computed DOM structure of the parsed cue text data. What I would think would need to happen for this bug to get done is to have the computed DOM structure generated lazily and have some kind of mechanism for marking dirty cue text that needs to be re-computed.
(Assignee)

Updated

5 years ago
Assignee: nobody → rick.eyre
Status: NEW → ASSIGNED
(Assignee)

Comment 2

5 years ago
Created attachment 768503 [details] [diff] [review]
v1: Implement TextTrackCue::DisplayState

I've renamed mCueDiv to mDisplayState to keep more inline with the spec. I've also introduced mReset in order to tell when we need to rebuild the computed cuetext of the TextTrackCue.

The code that will leverage this will land in bug 865407.
Attachment #768503 - Flags: review?(giles)
Comment on attachment 768503 [details] [diff] [review]
v1: Implement TextTrackCue::DisplayState

Review of attachment 768503 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

::: content/media/TextTrackCue.h
@@ +341,4 @@
>    int mLine;
>    TextTrackCueAlign mAlign;
>  
> +  // Holds the computed CSS boxes that represent the parsed cue text.

We're using this to attach elements, not Frames, so I don't think it's correct to call these 'computed CSS boxes'. Those are generated downstream by the layout engine.
Attachment #768503 - Flags: review?(giles) → review+
(Assignee)

Comment 4

5 years ago
Created attachment 768687 [details] [diff] [review]
v1: Implement TextTrackCue::DisplayState

Carring forward r=rillian.

- Updated mDisplayState's comment to reflect that we are generating DOM elements.
- Updated mReset's comment to reflect that it is set anytime any property that affects TextTrackCue's rendering is changed.
Attachment #768503 - Attachment is obsolete: true
(Assignee)

Comment 6

5 years ago
Try looks green. Should we get anyone else to review this Ralph?
Flags: needinfo?(giles)
(Assignee)

Comment 7

5 years ago
In our weekly meeting Ralph said that his reviewing of this was enough. Marking for checkin now.
Flags: needinfo?(giles)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/73f2aa8c286e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.