-
Notifications
You must be signed in to change notification settings - Fork 5
Make "Using computed attribute values" normative #256
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
Conversation
andreastai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks!
029f248 to
ad47c47
Compare
index.html
Outdated
| to the begin time of its parent element. If a <code><div></code> element specifies a <code>begin</code> | ||
| attribute, and that element does not correspond to a <a>Script Event</a>, then the computed times | ||
| of its descendant <code><div></code> elements that do correspond to <a>Script Events</a> | ||
| are different to what they would be if that intermediate <code><div></code> element's <code>begin</code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading this text again, I find the parts about "that element does not correspond to a Script Event" confusing. It shouldn't matter whether or not it is a Script Event. Inheritance applies. Also it gives the impression that under some circumstances, intermediate div can be ignored.
I would remove the whole sentence starting with "If". Otherwise, I would rephrase saying something like:
If a workflow decides to rewrite a DAPT document to remove nested div elements, care should be taken that if those elements had inheritable attributes like begin, the attribute values like begin on the leaf divs should be adjusted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the idea of making this example specifically about rewriting documents - it's about the correct interpretation of a document, regardless of what is being done with it.
Possibly this comment should be raised as a separate issue?
I'd be happy to emphasise further (informatively) that ignoring the intermediate <div> is not an acceptable thing to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion was a tentative rewrite of what I thought you were trying say. The alternative was to remove the sentence. My main problem is the current emphasis on elements that do not correspond to Script Events. We should not encourage that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's another attempt at what I was trying to say, maybe this is less confusing?
Or, for another example, the computed times of an element in a DAPT document are relative to the begin time of its parent element. If a
<div>element specifies abeginattribute, then the computed times of its child<div>elements are relative to that intermediate<div>element's begin time, and so on down the hierarchy. It is important to include those intermediate<div>elements' times in the computation even if the processing target is an instance of the DAPT data model in which they have no direct equivalent; otherwise the Script Event Begin and End times would be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change pushed in 33c33fe. On re-reading, I made a couple of small tweaks because I thought the concept of an "intermediate" div wasn't really explained properly.
ad47c47 to
b4a9806
Compare
|
The Timed Text Working Group just discussed
The full IRC log of that discussion<nigel> Subtopic: Make "Using computed attribute values" normative #256<nigel> github: https://github.com//pull/256 <cpn> Nigel: Andreas approved this PR. Cyril may be requesting changes <cpn> ... Your comment wasn't about a change introduced in the PR, about using computed attribute values <cpn> ... In section 6.4, there's a paragraph about computing times to div elements <cpn> ... I tried another rewrite. What to do? <cpn> Cyril: Your rewrite looks good <nigel> SUMMARY: @nigelmegitt to commit proposed change and @cconcolato to approve before merge <cpn> Nigel: Any other comments on this? <cpn> (nothing) |
Retain and reformat the example about non-DAPT functionality.
As per review comment and discussion.
67d6319 to
d0e3e3e
Compare
cconcolato
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I'm approving the PR to unlock the CR.
That said, I don't think it is a substantive change because the changes are about examples. The section contains a single MUST statement that is not changed.
Anyway, I think we could improve the section editorially later on. I find it scary for implementers because the boundaries of the implementation requirements are not clear. There are many examples (xml:lang, begin with nested divs, styles), but the concrete normative text is not visible enough, upfront. As I understand it, there are only 2 concrete consequences on implementers:
- use of style TTML2 properties, and there are 2 cases: either the implementation cares about them and it needs to implement proper inheritance; or it does not care about them and can fully ignore them
- use of nested divs (which I'm not a big fan of...). Implementations don't have a choice. To be compliant, they have to apply the inheritance model on intermediate divs.
Thanks @cconcolato - the reason the section was made normative was because it had a MUST statement in, but was previously marked as non-normative.
Not just style properties and nested divs, also timing properties and anything else that can be put on a div, like Thanks for the approval, I will merge, and if more problems or improvements are identified they can be dealt with in new issues and pull requests. |
Closes #249.
Retain and reformat the example about non-DAPT functionality.
Preview | Diff