search

w3c / ttml2

Timed Text Markup Language 2 (TTML2)
https://w3c.github.io/ttml2/
Other
41 stars 16 forks source link

The set element is included in [resolve computed styles]. #950

Open palemieux opened 6 years ago

palemieux commented 6 years ago

The specified style set (SSS) of a set element is merged into the SSS of its parent (Steo 10.4.4.2(5)). As a result, the computed style set (CSS) of a set element should not be generated, and set elements should be excluded from step 10.4.4.4(2) as they were in TTML1, i.e. set should be removed from the list at step 10.4.4.4(1).

skynavga commented 6 years ago

There is no problem here. The reason for this is as follows: if E is animate or set, then CSS(E) := SSS(E). Furthermore, in this case, SSS(E) is a subset of what would normally be the case for other elements, since neither inheritance nor initial value fallback applies. These semantics follow from the preamble of 10.4.4.2 step (6) and 10.4.4.3 step (3), the relevant parts of which I quote below:

10.4.4.2 step (6)

[implicit inheritance and initial value fallback] if the element type of E is not animation element type animate or set

10.4.4.3 step (3)

if E is an animate, set, or style element, then return CSS(E) as the resulting computed style set without further resolution

My conclusion is that, since we already very carefully crafted language to handle the case of styles on animate and set, that no change is required.

palemieux commented 6 years ago

There is no problem here. The reason for this is as follows: if E is animate or set, then CSS(E) := SSS(E).

This is not relevant since 10.4.4.2(5) merges the SSS of set into the parent, and not the CSS of set .

Are you arguing that the current CSS computation on set is a no-op because it is never used to set or compute styles on the parent?

skynavga commented 6 years ago

Are you arguing that the current CSS computation on set is a no-op because it is never used to set or compute styles on the parent?

Yes, that is basically correct. 10.4.4.2(5) is rather unusual because style resolution is occurring in pre-order traversal. That means that the parent of an inline animation is reaching down to its children animations and asking for each of their SSS(A), but, from the traversal order, their SSS(A) has not yet been computed. However, the definition of SSS(A) effectively requires performing all steps of 10.4.4.2, which is, itself, in the scope of 10.4.4.3(1), which is in the scope of 10.4.4.4(1).

So, you have the following happening (in order):

  1. resolve SSS(PARENT(A)) 10.4.4.2
  2. merge SSS(A) into SSS(PARENT(A)) 10.4.4.2(5)
  3. resolve SSS(A) 10.4.4.2
  4. continue pre-order traversal
  5. resolve CSS(A) 10.4.4.3(2), which reuses results SSS(A) from (3) above

So, from this, we eventually get full computation of CSS(PARENT(A)), and we also get CSS(A), which is exactly SSS(A) as currently defined.

You might ask, why bother even creating CSS(A)? It is because when we write out an ISD we may choose to write out inline animations and have them reference their CSS(A) which had been serialized using an isd:css (or equivalent).

Finally, it would take some work to back out this logic and we may get it wrong. So I'm inclined to leave it as is since it is self-consistent and implemented.

css-meeting-bot commented 6 years ago

The Working Group just discussed The set element is included in [resolve computed styles]. ttml2#950, and agreed to the following:

The full IRC log of that discussion <nigel> Topic: The set element is included in [resolve computed styles]. ttml2#950
<nigel> github: https://github.com/w3c/ttml2/issues/950
<nigel> Nigel; [attempts to summarise issue] This is in the review scope for the CfC.
<nigel> Glenn: [scribe misses a bit] The CSS of set and animate are defined as the SSS without
<nigel> .. inheritance or fallback, as documented more thoroughly. It is very dense logic to follow.
<nigel> .. The upshot is there is no need to exclude set or animate from the filter set to which
<nigel> .. CSS computation applies. If we did not have it then set and animate would have no
<nigel> .. CSS applied on them at all but there are other parts of the spec that want to have that.
<nigel> .. The current wording avoids any substantive issue, the CSS is equal to the SSS basically.
<nigel> Nigel: Pierre, does that logic resolve it for you?
<nigel> Pierre: I have not studied it further.
<nigel> Glenn: If we pull out set from that list then we would also pull out animate. At least three
<nigel> .. rules with special semantics for set and animate would have to be backed out as well.
<nigel> .. It would be very tricky making those changes now for limited utility. I am willing to
<nigel> .. leave this issue open and give further consideration. I don't think we can address it in
<nigel> .. TTML2 1st Edition in any case. I don't see a bug here.
<nigel> Nigel: I think at this stage there is no change proposed and it will be difficult to make one.
<nigel> .. In order to make the transition request our realistic options are to defer it or close it.
<nigel> Glenn: I would prefer to defer it to TTML.next.
<nigel> Pierre: I think the group needs to decide whether to close it or not.
<nigel> .. I don't know if I would object to closing it.
<nigel> Nigel: We need a default action to take here. If it is not closed and no change is agreed
<nigel> .. by the end of the CfC then I will defer it, so we can move on with the transition request.
<nigel> SUMMARY: Issue review to continue; to defer if not resolved by the end of the CR3 CfC.
skynavga commented 6 years ago

Per resolution, deferring by marking as ttml.next and closing. May be reopened for TTML2 2e processing.

skynavga commented 5 years ago

Removing ttml.next, leaving here for ttml2 2e processing.

nigelmegitt commented 5 years ago

Adding to agenda for this week's call - not sure if we need to re-open this to address it.

skynavga commented 5 years ago

Having just re-reviewed this issue, my opinion has not changed, which is, there is not a problem to solve here. I recommend we close this without further action, that is, unless someone can produce a test file that demonstrates how following the prescribed algorithm is broken in some deterministic fashion.

css-meeting-bot commented 5 years ago

The Timed Text Working Group just discussed The set element is included in [resolve computed styles]. ttml2#950, and agreed to the following:

The full IRC log of that discussion <nigel> Topic: The set element is included in [resolve computed styles]. ttml2#950
<nigel> github: https://github.com/w3c/ttml2/issues/950
<nigel> Nigel: [Summarises older comments on issue] It wasn't clear if any change was being proposed, one option is to close it.
<nigel> Glenn: If anyone can provide test content showing the current text is broken then we could reopen it.
<nigel> Pierre: We are also doing editorial improvements, so this might fit in that category.
<nigel> Glenn: I marked it as substantive because it requires changing the substance of one of the algorithms.
<nigel> .. As you've asked for, with any substantive change we should have test material.
<nigel> Pierre: It may be that the outcome of the algorithm is the same, as you already argued, so in that case it could be editorial.
<nigel> .. An editorial clarification perhaps.
<nigel> Glenn: I haven't seen any proposed language for that, if you would like to propose a note that would clarify something
<nigel> .. for the reader, I would be happy to consider that and downgrading this to be editorial.
<nigel> Pierre: Happy to be assigned this and I will follow up on it.
<nigel> q?
<nigel> SUMMARY: Pierre to consider if any editorial change would improve this, assigning to @palemieux
<nigel> Glenn: Just to be clear, I would view removing set from the list in 10.4.4.4 as a substantive change, that's definitely
<nigel> .. changing the algorithm.
<nigel> Pierre: Not if the outcome is the same as you argue.
<nigel> Nigel: Let's not argue hypotheticals here, wait until we have a concrete proposal.
<nigel> Glenn: By the way, in one of the comments in the thread on this issue I pointed out that computing the SSS of the set
<nigel> .. does have a substantive effect if you're serialising the CSS into an ISD. From a presentation only perspective if you
<nigel> .. are not considering an ISD there may be no semantic difference but we also have the ISD mechanism to keep in mind.
palemieux commented 5 years ago

It is because when we write out an ISD we may choose to write out inline animations and have them reference their CSS(A) which had been serialized using an isd:css (or equivalent).

No set element is present in ISDs per J.1.4

skynavga commented 5 years ago

@palemieux thanks for bringing that to my attention, which would appear to invalidate my premise on insisting on keeping set in the list under 10.4.4.4(2); let me verify this further over the next few days, and, if your point bears out and there is no other use for CSS(set), then I will be open to removing it (as a substantive, but non-testable change), in which case I can proceed with creating a PR;

skynavga commented 5 years ago

After further review, I have determined that we cannot remove set from 10.4.4.4(1) [filter]. The reason we can't do this is that we must invoke 10.4.4.4(2) [resolved computed styles] on set in order to obtain SSS(set) via 10.4.4.3(1). While it is true that 10.4.4.3(2) does create CSS(set) and initializes it to SSS(set), 10.4.4.3(3) prevents further relative value resolution for CSS(set). Since such CSS(set), as well as CSS(animate) and CSS(style), are not used elsewhere, an implementation is free to discard or even not go to the trouble of creating such CSS instances. This is entirely consistent with the language in the preamble of §10.4, namely

Any implementation of this model is permitted provided that the externally observable results are consistent with the results produced by this model.

skynavga commented 5 years ago

I recommend this issue be closed with no action.

css-meeting-bot commented 5 years ago

The Timed Text Working Group just discussed TTML2 issue 950.

The full IRC log of that discussion <cyril> Topic: TTML2 issue 950
<cyril> glenn: my only point is that I added one more comment
<nigel> github: https://github.com/w3c/ttml2/issues/950
<cyril> ... we cannot remove the set element, my explanation is there
<cyril> ... I verified that the algorithm needs it
palemieux commented 5 years ago

this is that we must invoke 10.4.4.4(2) [resolved computed styles] on set in order to obtain SSS(set) via 10.4.4.3(1).

@skynavga, Step 10.4.4.2(5) is the step that handles computing set style values, by including these style values in the specified style set of the parent.

skynavga commented 5 years ago

[deleted]

skynavga commented 5 years ago

If we should remove set and animate from the list in 10.4.4.4(1), then how do you propose we test this change? I can't see how it is testable. Furthermore, removing these items is really nothing more than an (optional) optimization, which an implementation can already make without being non-compliant. So, the bottom line is: do we really need to make this change? My answer is no, we do not.

skynavga commented 5 years ago

Added to TPAC Agenda, https://www.w3.org/wiki/TimedText/tpac2019#Topics.

palemieux commented 5 years ago

@skynavga In TTML1, the set element was not listed in 10.4.4.4(1), and I ran into this issue while adding support for TTML2 to imscJS. If set was added to 10.4.4.4(1) without any specific reason, then it should be removed so that (a) other implementers do not run into the same question and (b) to avoid other (at this point unknown) consequences.

css-meeting-bot commented 5 years ago

The Timed Text Working Group just discussed The set element is included in [resolve computed styles]. ttml2#950, and agreed to the following:

The full IRC log of that discussion <nigel> Topic: The set element is included in [resolve computed styles]. ttml2#950
<nigel> github: https://github.com/w3c/ttml2/issues/950
<cyril> glenn: after discussions, I said we could remove it
<cyril> pal: I think it was added as an error when going from TTML1 and TTML2 and so it should be removed
<cyril> glenn: we could not remove set and not remove animate
<cyril> nigel: Pierre says is an editorial and Glenn says it can't be tested
<cyril> pal: it was introduced by mistake and we should remove it
<cyril> glenn: I assume it was not an accident
<cyril> ... if we cannot test the removal, why remove it given the editorial pain
<cyril> pal: the goal was not to create a divergence between TTML1 and TTML2
<cyril> ... it's only a spec divergence
<cyril> ... I checked the differences in algorithms in TTML1 and TTML2 and this one stood out
<cyril> glenn: it's a change in a normative section
<cyril> cyril: can we ask to go through the editing work to see if there is any problem?
<cyril> glenn: I think we have done that
<cyril> ... and I said I am ready to remove them
<cyril> pal: let's keep the issue open
<cyril> glenn: fine
<cyril> SUMMARY: defer this for the time being and don't make this a dependency for TTML2 2nd edition, removed from the milestone