search

w3c / webvtt

WebVTT Standard
https://w3c.github.io/webvtt/
Other
104 stars 40 forks source link

Added unbounded TextTrackCue.endTime #493

Closed rjksmith closed 3 years ago

rjksmith commented 4 years ago

Added support for unbounded TextTrackCue - see https://github.com/whatwg/html/pull/5953 Whitespace removed by Atom

Preview | Diff

himorin commented 3 years ago

@himorin I've signed the non-participant agreement but still see a failing check on this issue.

sorry on late reply. it takes some time (depends on health and delays - periodical processing of many of back end services) for IPP, and please count some time for these... (and seems fine for now on IPP / ipr.)

rjksmith commented 3 years ago

@gkatsev whatwg/html#5953 and web-platform-tests/wpt#28394 have been merged. As mentioned above, Web Platform Tests rely on VTTCue and so will fail spuriously without this PR. Please now review and merge #493 to avoid a mismatch with WPT and let me know if there are any problems. Thanks.

chrisn commented 3 years ago

This change looks good to me, and seems straightforward to align with the TextTrackCue change in HTML. Is it OK to merge? Can the questions around the WebVTT document format be followed up separately to merging this change?

chrisn commented 3 years ago

Thanks @rjksmith and thanks @foolip for helping out!

I believe this is good to merge and the question of updating the syntax to allow unbounded endTime has been moved to #496. It's on the agenda for this Thursday's TTWG meeting.

Thanks @gkatsev :-) I'll join the meeting tomorrow.

rjksmith commented 3 years ago

I'm happy to join too if someone can send me an invite. Thanks.

css-meeting-bot commented 3 years ago

The Timed Text Working Group just discussed WebVTT Add unbounded TextTrackCue.endTime w3c/webvtt#493, and agreed to the following:

The full IRC log of that discussion <nigel> Topic: WebVTT Add unbounded TextTrackCue.endTime w3c/webvtt#493
<nigel> github: https://github.com/w3c/webvtt/pull/493
<nigel> Gary: When we last spoke about the unbounded end time for the TextTrackCue there wasn't much concern.
<nigel> .. The hold-up was whether there were tests and whether it was approved on the HTML spec side.
<nigel> .. Last week the [PRs on] tests and the HTML side were merged so now WebVTT is the last hold-out for the API change,
<nigel> .. which is this pull request right now.
<RobSmith> q+
<nigel> ack R
<nigel> Rob: I concur. Firstly this is my first TTWG meeting, thank you for the invite. Thanks Gary for the quick summary.
<nigel> .. I'd add that having done the platform tests I notice there's a dependency between the TextTrackCue tests and the VTTCue tests
<nigel> .. because the TextTrackCue relies on VTTCue for the constructor. There's a crossover or interdependency that results.
<nigel> .. The WebVTT change is required to make the Web Platform Tests work as well.
<nigel> Gary: Thanks Rob that makes sense.
<nigel> .. In terms of process, I haven't heard any objections from anyone. Do we need to wait before merging for the resolution period?
<nigel> Nigel: Our normal practice is to consider the opening of a PR as effectively a Call for Consensus
<nigel> .. so that the Decision Review period expires 2 weeks after opening, obviously assuming there is consensus at that time.
<nigel> .. What this means is that, since this PR has been open since November, if the Chair declares consensus, then the Editor can go ahead and merge.
<RobSmith> q+
<nigel> .. I'd just point out that there was a commit just 23 hours ago so that should be considered.
<nigel> Gary: I plan to look through it again after this meeting and in that case go ahead and merge.
<nigel> ack R
<cpn> q+ to ask about the VTT format discussion
<nigel> Rob: That recent addition was to address Philip Jaegenstadt's comment that there was a reference to unbounded TextTrackCue which was
<nigel> .. not used. We tried to keep it as simple as possible and ended up removing all references to unbounded cues in the process.
<nigel> .. The way it is at the moment it is the one thing we are adding and do not mention, so I thought it worth adding.
<cpn> s/Jaegenstadt/Jaegenstedt/
<nigel> .. I couldn't find any similar example and struggled with the wording so would welcome guidance.
<nigel> Gary: Earlier I thought it was fine but will take a closer look.
<nigel> Rob: Thank you
<nigel> q++ nigel
<nigel> ack n
<nigel> ack +
<nigel> Gary: Then for the pull request the resolution is for me to look over it and merge after approval.
<RobSmith> q+
<cpn> q- cpn
<nigel> ack Rob
<nigel> Rob: Gary, if there's change to be made on the wording I'm happy to work with you to get that resolved and complete this.
<nigel> Gary: Thank you Rob
<nigel> RESOLUTION: Gary to do final Editorial pass and merge
rjksmith commented 3 years ago

@gkatsev Fantastic. Many thanks for your swift response. This enables the WPT update in web-platform-tests/wpt#28394 which allows browser implementers to validate their code for unbounded cue support, and concludes the changes required for whatwg/html#5953.

I look forward to the constructive discussion of possible changes to WebVTT in w3c/webvtt#496.