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

clarify what spans are possible in a text and how they are handled #158

Merged
merged 2 commits into from
Mar 1, 2024

Conversation

cconcolato
Copy link
Contributor

@cconcolato cconcolato commented Jun 14, 2023

close #17


Preview | Diff

@cconcolato cconcolato requested a review from nigelmegitt June 14, 2023 18:06
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated
@@ -923,7 +923,7 @@ <h4>Text</h4>
<p>A <a>Text</a> object is represented in a <a>DAPT Document</a> by a <code>&lt;p&gt;</code> element with the following constraints:</p>
<ul>
<li>The <a>Text</a> of the <a>Script Event</a> is represented by the character content
of the <code>&lt;p&gt;</code> and its descendants, and after applying White Space Handling
of the <code>&lt;p&gt;</code> and of its descendants, and after applying White Space Handling
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really sorry, I don't know why I'm thinking of these issues one at a time rather than all together, but there's another issue: not all of its descendants' character content is the Text - for example any descendant of a <metadata> child would not count.

I'm not sure quite how to word it, because <span>s can have nested <span> children themselves, and so on, but it would also be possible (with undefined semantics) to have a <span> descendant of a <metadata> element or some other foreign namespace element.

There is probably a better way, but here's what I came up with:

Suggested change
of the <code>&lt;p&gt;</code> and of its descendants, and after applying White Space Handling
of the <code>&lt;p&gt;</code> element
and of its <code>&lt;span&gt;</code> element children
and (recursively) their <code>&lt;span&gt;</code> element children,
and after applying White Space Handling

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about <br> elements?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, how about inserting before ", and after applying White Space Handling" an additional line:

, where &lt;br&gt; elements are considered to represent new lines within the character content

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about ruby annotation text? How should we consider it? Should we be silent about it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can be silent about it because ruby annotation text is part of the Text, and the mechanism for inserting it uses nested <span>s whose content is already included in the definition. Unless your view is that ruby annotations are not part of the Text, @cconcolato ?

Copy link
Contributor Author

@cconcolato cconcolato Feb 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to address your comments (also squashed and rebased against the main branch to resolve conflicts)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that 0512851 to resolve #158 (comment) ? I don't think that wording rejects character content children of <span> elements inside foreign namespace elements - I think we agree that such content should not be included in the definition of Text?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added pruning (but it probably should be rephrased even more, as pruning should happen first).

I'm not convinced we need all of that text. So far, the spec is written using the following structure:
"The [part of the DAPT model] is represented by [some syntax in TTML]"
I think it is expressing a one way mapping: how from the model you build the syntax, not how from the syntax you obtain the model.

Maybe we want that bidirectional mapping? That would require reviewing the rest of the spec (e.g. what if the TTML elements representing a Character, contain more than the few elements specified in the spec).

Assuming, the mapping is one way for now (for consistency with other parts of the spec), the text in this PR should only say that the Text object is represented by a p element, possibly structured in spans, possibly with timing, possibly with interspersed metadata and foreign elements. All the "possibly" parts are covered by notes already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nigelmegitt I would be interested in your feedback on the above comment.

Copy link
Contributor

@nigelmegitt nigelmegitt Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still thinking about this!

My instinct is that it's fine as is, and maybe we should open a separate issue about this question. I think the key question for implementers is what they should do if they encounter a TTML2 document that claims to be a DAPT document but doesn't exactly match the data model. Are we telling them to implement all relevant TTML2 semantics or only those specifically mentioned in DAPT? Formally I believe it is the set of features (TTML2 + extensions) defined as being required for processors. From that perspective I don't believe we need to say any more about it.

But happy to discuss further.

@css-meeting-bot
Copy link
Member

The Timed Text Working Group just discussed clarify what spans are possible in a text and how they are handled w3c/dapt#158, and agreed to the following:

  • SUMMARY: @cconcolato to attempt a further edit
The full IRC log of that discussion <nigel> Subtopic: clarify what spans are possible in a text and how they are handled #158
<nigel> github: https://github.com//pull/158
<cpn> Cyril: Should we go back to the original wording?
<cpn> Nigel: Any recommendations for forms of words would be welcome!
<cpn> Cyril: I looked at the original issue #17, the recommendations were different to what we landed with
<cpn> ... It was about spans with specific timing, so a different issue
<cpn> Nigel: The PR does include spans with timing, does address that issue
<cpn> ... But it also adds something about text of script events
<cpn> ... We discussed back in June
<cpn> Cyril: I think its because we're trying to define what text content means
<cpn> ... I fear we're going into a spiral of adding more
<cpn> ... I can try to add spans in metadata or foreign elements are not considered
<nigel> SUMMARY: @cconcolato to attempt a further edit

Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this works for me.

@nigelmegitt nigelmegitt added the agenda Issue flagged for in-meeting discussion label Feb 24, 2024
@cconcolato cconcolato merged commit 2cb90b5 into main Mar 1, 2024
2 checks passed
@cconcolato
Copy link
Contributor Author

As discussed in the TTWG call https://www.w3.org/2024/02/29-tt-minutes.html#t03, a new issue #214 was created and this PR can be merged.

@cconcolato cconcolato deleted the adaptation branch March 1, 2024 04:50
@nigelmegitt nigelmegitt removed the agenda Issue flagged for in-meeting discussion label Mar 12, 2024
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.

Absence of adaptation information.
3 participants