Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New snapshot with new at-risk items #460

Open
wants to merge 20 commits into
base: gh-pages
Choose a base branch
from

Conversation

gkatsev
Copy link
Collaborator

@gkatsev gkatsev commented Jun 19, 2019

@gkatsev
Copy link
Collaborator Author

gkatsev commented Jun 19, 2019

The changes and diff docs still need to be updated, I'll make sure to document how to generate those so it's easier to do it in the future.

@gkatsev
Copy link
Collaborator Author

gkatsev commented Jun 24, 2019

This would require the changes in #461.

@css-meeting-bot
Copy link
Member

The Timed Text Working Group just discussed New snapshot with new at-risk items webvtt#460.

The full IRC log of that discussion <nigel> Topic: New snapshot with new at-risk items webvtt#460
<nigel> github: https://github.com//pull/460
<nigel> Gary: This has the change for making the region lines a regular long from unsigned long
<nigel> .. as well as the change to clamp the lines property to the closest value that can be represented
<nigel> .. It also adds the at-risk items.
<nigel> Nigel: Looks like the last significant change was 14 days ago and the positive review was older.
<nigel> Gary: Yes, this incorporates changes that Silvia has also approved from other PRs.
<nigel> .. The biggest change is the rounding, where I looked at the HTML spec to see how to do it and want someone
<nigel> .. else to look at it and check it's okay.
<nigel> Nigel: Nobody has done that yet?
<nigel> Gary: Silvia approved #470 which was the PR that introduced this, nobody else has.
<nigel> .. If we're okay with just Silvia then we can go ahead.
<nigel> Nigel: #470 was 17 days ago and Silvia approved it the same day.
<nigel> .. In this situation it has had 2 weeks and a positive review so we normally say go ahead and do it.
<nigel> Gary: Awesome I'll go ahead then.
<nigel> Nigel: What's the state of the tests for this?
<nigel> Gary: I need to update the Lines PR to match it, to split the rounded and clamped/unclamped values but now we don't
<nigel> .. need to verify the unclamped values and can delete that portion.
<nigel> .. I was going to use the CR time to look through and fix VTT.js implementation and whatnot to get the other things in
<nigel> .. the IR to pass.
<nigel> Nigel: From our normal process we can just go ahead at this point. Any objections?
<nigel> Nigel: Silence, so go ahead.
<nigel> Gary: Then I'll proceed, thanks.

@silviapfeiffer
Copy link
Member

You got a double up of two headings for "former editors". Looking good otherwise.

@gkatsev
Copy link
Collaborator Author

gkatsev commented Aug 7, 2019

@plehegar @himorin so, I think this PR is good to go. Should I change the snapshot date to a recent date? What's the process to get this published?

@himorin
Copy link
Contributor

himorin commented Aug 21, 2019

@plehegar @himorin so, I think this PR is good to go. Should I change the snapshot date to a recent date? What's the process to get this published?

sorry for getting to this late (for off week..). I actually need to get confirmation from @plehegar whether I am not doing wrong, but in my understanding

  • finish merging PR(s) with the date in document(s) as target publication day
  • pushing new document using publication system (by myself)

@gkatsev
Copy link
Collaborator Author

gkatsev commented Aug 27, 2019

@himorin thanks! I updated the dates to be September 1 (27db78f). If those dates work, then this PR is can be merged by anyone and published.

@himorin
Copy link
Contributor

himorin commented Aug 31, 2019

I've got comment and corrections of mistakes, and I realized we need to follow section 6.4.1 of the Process document, as updating CR with substantive changes.
(Just to confirm, but this update includes substantive change, right?)

Let me check charter for wide reviews, and work on preparation of transition request.

@gkatsev
Copy link
Collaborator Author

gkatsev commented Sep 3, 2019

Yeah, I guess this does have substantive changes, given the changes to VTTRegion lines.

@himorin let me know if there's more work needed on my end.

@himorin
Copy link
Contributor

himorin commented Sep 4, 2019

@gkatsev I've put draft of transition request (one in email conversation, but updated some part after discussion with @plehegar ) at w3c/transitions#160, please have a look and give some comment (or edit it) on several points with ? or 'comment required'.

@gkatsev
Copy link
Collaborator Author

gkatsev commented Sep 4, 2019

@himorin Thanks for your help. Looks like I was a bit premature in request a transition. I had assumed that everyone's agreement on the snapshot meant we could proceed with the CR, but we need to go through the CfC process.

@css-meeting-bot
Copy link
Member

The Timed Text Working Group just discussed Discussion snapshot w3c/webvtt#460.

The full IRC log of that discussion <cyril> Topic: Discussion snapshot #460
<nigel> github: https://github.com//pull/460
<cyril> gkatsev: there is a bunch of changes in the snapshot
<cyril> ... I wanted to go through and categorize them according to the W3C Process document
<cyril> nigel: which one is the right list?
<cyril> gkatsev: changes.html
<gkatsev> -> https://htmlpreview.github.io/?https://github.com/gkatsev/webvtt/blob/at-risk-june/archives/2019-09-01/changes.html list of changes
<cyril> nigel: normally the technical changes would be the substantive one
<cyril> gkatsev: we can review the at-risk ones
<cyril> nigel: I'd be interested to a staff view on that
<cyril> gkatsev: the "make vtt lines be a long" one is changing the WebIDL for the lines property from an unsigned long to a long
<cyril> nigel: it has 2 GitHub issues
<cyril> gkatsev: one is the PR and the other one is the issue
<cyril> ... 457 is the issue
<cyril> ... and 461 fixes 457 as well
<cyril> nigel: the change document lists the github issues, so you might want to change the first one to 457 and remove the editorial dup
<cyril> gkatsev: yes, I'll do that
<cyril> nigel: that is indeed substantive
<cyril> gkatsev: from a process perspective, that is a correction that does not add new features
<cyril> gkatsev: next one is "Update region lines parsing to round to +/- MAX_VALUE"
<cyril> ... it's based on the current implementation in Safari
<cyril> ... the mozilla people are on board with that change
<cyril> ... it is a correction that does not add new features
<cyril> nigel: yes, that is not a new feature
<cyril> ... but it is definitely substantive
<cyril> ... it's worth pointing to the issue 467
<cyril> gkatsev: I'll do that for the at risk ones also
<cyril> ... next one is changing editors
<cyril> nigel: obviously editorial
<cyril> ... then you have the at risk ones
<cyril> ... and the last was a dup, we discussed it
<cyril> ... so there was no change made for the [IR] issues?
<cyril> gkatsev: a lot of them are already covered by these changes
<cyril> [going through the IR issues]
<nigel> I'm on #465
<cyril> gkatsev: Safari for this one, the region parser is correct, but the way you interact is not
<cyril> nigel: are they going to fix that?
<cyril> gkatsev: I hope so
<cyril> nigel: I mean during the CR period
<cyril> ... you could get stuck in CR because of this
<cyril> ... one route out could be for Safari to remove the prefix
<cyril> ... another route is to mark it at risk
<cyril> ... another one would be to tweak the test
<cyril> ... on the last one I'm a bit uneasy because we would not be testing the spec
<cyril> gkatsev: if it's parsing correctly, that means the test should pass
<cyril> ... but it's failing after it's parsing
<cyril> nigel: we have members from apple in this group
<cyril> ... we should be going to them
<cyril> ... Eric would be a good person
<cyril> ... asking: is this something they might fix in the developer preview
<cyril> gkatsev: I'll reach out to them
<cyril> ... 463 is similar, weird implementation in Safari
<cyril> nigel: the resolution for this one is not marked at risk
<cyril> ... so what's our resolution
<cyril> gkatsev: we should talk to Apple
<nigel> s/resolution/exit plan
<cyril> ... we could mark regions at risk but removing it would not be good
<cyril> nigel: depending on their response, we may need to tweak the spec or just wait until they are done with it
<cyril> nigel: the last one is 464
<cyril> ... entities test failing
<cyril> ... we don't have an exit plan for this issue
<cyril> ... if this continues, do we have any other implementation that would pass?
<cyril> gkatsev: I think vtt.js may support it
<cyril> ... also we can try doing what we discussed the lines attribute, split up the test in 2 portions
<cyril> ... and say the core part passes
<cyril> ... but the first thing is to see how vtt.js fares
<cyril> ... I could tweak it
<cyril> nigel: we have a clearer understanding about the snapshot
<cyril> ... there is some action needed because we don't want to go back to CR again
<cyril> Xabier: when you speak about regions, are they CSS regions?
<cyril> gkatsev: no, WebVTT regions
<cyril> nigel: it's a good question, what's the relationship between the 2
<cyril> calvaris: is there are relationship?
<cyril> gkatsev: I don't think so

combine the two VTTRegion issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mark text-combine-upright as at risk Mark text-wrap: balance as at risk
4 participants