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

<set> elements no longer pruned by ISD #276

Merged
merged 4 commits into from
Dec 18, 2017
Merged

Conversation

palemieux
Copy link
Contributor

Closes #216

@palemieux palemieux added this to the 3rd Ed milestone Dec 5, 2017
@palemieux palemieux self-assigned this Dec 5, 2017
@skynavga skynavga self-requested a review December 7, 2017 05:18
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.

I've asked a question - it seems that a further change is probably needed to resolve the issue.

spec/ttml1.xml Outdated
@@ -7405,7 +7405,8 @@ headed by the <el>body</el> element;</p>
</item>
<item>
<p>evaluating this sub-tree in a postorder traversal, prune elements if they
are not a <loc href="#element-vocab-type-content">Content</loc> element, if they are temporally inactive, if they are empty,
are not a <loc href="#element-vocab-type-content">Content</loc> or <loc href="#element-vocab-type-animation">Animation</loc> element,
if they are temporally inactive, if they are empty,
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "if they are empty" mean in this context? Now that we are trying to avoid pruning set elements, I think we need to change this "if they are empty" clause as well. Looking at https://www.w3.org/TR/ttml1/#animation-vocabulary-set the only contents permitted on a <set> are metadata; consequently it is likely that well formed <set> elements that should do something would be considered empty and therefore would be pruned, in this case unintentionally.

One solution to this would be to change the phrase to "if they are an empty Content element". Another would be to adopt the TTML2 approach of defining presentation related element and referencing that term. The term would have to be defined a little differently because some classes of element referenced by the term definition in TTML2 are not present in TTML1.

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, looks good to me.

@palemieux palemieux merged commit 371d8a0 into master Dec 18, 2017
@palemieux palemieux deleted the issue-216-do-not-prune-set branch December 18, 2017 18:53
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.

None yet

3 participants