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

Prohibit usesForced metadata item #314

Merged
merged 4 commits into from Feb 14, 2018

Conversation

@palemieux
Copy link
Contributor

palemieux commented Jan 29, 2018

Closes #311

@palemieux palemieux added the imsc1.1 label Jan 29, 2018
@palemieux palemieux self-assigned this Jan 29, 2018
@palemieux palemieux requested a review from nigelmegitt Jan 29, 2018
@palemieux palemieux added pr open and removed pr open labels Jan 29, 2018
Copy link
Contributor

nigelmegitt left a comment

Needs to be weakened - I can't accept this as is, since it is too draconian to prohibit any particular named metadata item, and unnecessary.

<section>
<h4><code>#item-metadata</code></h4>

<p>The <code>usesForced</code> named metadata item SHALL NOT be present.</p>

This comment has been minimized.

Copy link
@nigelmegitt

nigelmegitt Jan 29, 2018

Contributor

This is too strong. We should not be prohibiting any named metadata items, but it is reasonable to include a note somewhere that no processing semantic is based on metadata items including usesForced, and that the #forcedDisplay feature must be used for forced display semantics.

This comment has been minimized.

Copy link
@palemieux

palemieux Jan 29, 2018

Author Contributor

@nigelmegitt usesForced is defined as A boolean that expresses whether some expression makes use of the forced bound parameter, where the value adheres to xsd:boolean. usesForced cannot be present since condition is prohibited. So why not also prohibit usesForced to avoid any confusion?

This comment has been minimized.

Copy link
@nigelmegitt

nigelmegitt Jan 29, 2018

Contributor

See the related comment on #311 - no point having this discussion in two places at once.

@palemieux palemieux added the agenda label Feb 6, 2018
@palemieux

This comment has been minimized.

Copy link
Contributor Author

palemieux commented Feb 9, 2018

Modified PR per #311 (comment)

Copy link
Contributor

nigelmegitt left a comment

For me, this can be merged as is. I've also made a question/proposal for an additional clarifying sentence which I think would help explain why this is a SHOULD not a SHALL, for the curious.

<p>The <code>usesForced</code> named metadata item SHOULD NOT be present.</p>

<p class='note'>This specification prohibits the <code>condition</code> attribute. As a result, the <code>usesForced</code>
named metadata item does not apply, even if <code>itts:forcedDisplay</code> is used.</p>

This comment has been minimized.

Copy link
@nigelmegitt

nigelmegitt Feb 9, 2018

Contributor

Is it worth appending:

In particular, using a usesForced named metadata item set to true would be an error in TTML2.

?

@palemieux palemieux removed the agenda label Feb 9, 2018
@palemieux

This comment has been minimized.

Copy link
Contributor Author

palemieux commented Feb 12, 2018

@nigelmegitt Can you review the revised text?

Copy link
Contributor

nigelmegitt left a comment

Almost there...

<p>The <code>usesForced</code> named metadata item does not apply since the <code>condition</code> attribute is
prohibited.</p>

<p>The <code>itts:forcedDisplay</code> attribute is used instead to specify forced content semantics.</p>

This comment has been minimized.

Copy link
@nigelmegitt

nigelmegitt Feb 14, 2018

Contributor

Please remove the word "instead", since it implies that itts:forcedDisplay somehow has equivalent functionality to usesForced, which is not the case.

@palemieux palemieux merged commit eeaec52 into IMSC1.1 Feb 14, 2018
1 check passed
1 check passed
ipr PR deemed acceptable.
Details
@palemieux palemieux deleted the issue-311-prohibit-usesForced-metadata-item branch Feb 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.