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

Constrain text emphasis position to 'outside' #16

Merged
merged 2 commits into from
Apr 26, 2018

Conversation

palemieux
Copy link
Contributor

Close #15

@palemieux palemieux self-assigned this Jan 15, 2018
@palemieux palemieux added this to the v1.1 milestone Jan 18, 2018
@@ -159,7 +159,7 @@ <h3>Japanese Text Support</h3>
<tr>
<td><code>#textEmphasis-minimal</code></td>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there now a more expressive TTML2 feature designator we can use in place of this?

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 What do you mean by "more expressive"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean one that more accurately maps to support for/use of the outside value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None of the emphasis features relate specifically to the <emphasis-position> component of tts:textEmphasis

Copy link
Contributor

@nigelmegitt nigelmegitt Mar 13, 2018

Choose a reason for hiding this comment

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

Oh, I see that the #textEmphasis-no-XXX features are negative features rather than positive ones. I didn't notice that before. I think it's broken in TTML2 and will raise an issue for this: features should be positively stated not negatively, otherwise the future introduction of additional syntax and semantics will change the meaning of existing -no-XXX style feature designators.

Raised as w3c/ttml2#697.

@@ -159,7 +159,7 @@ <h3>Japanese Text Support</h3>
<tr>
<td><code>#textEmphasis-minimal</code></td>
Copy link
Contributor

@nigelmegitt nigelmegitt Mar 13, 2018

Choose a reason for hiding this comment

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

Oh, I see that the #textEmphasis-no-XXX features are negative features rather than positive ones. I didn't notice that before. I think it's broken in TTML2 and will raise an issue for this: features should be positively stated not negatively, otherwise the future introduction of additional syntax and semantics will change the meaning of existing -no-XXX style feature designators.

Raised as w3c/ttml2#697.

index.html Outdated
@@ -159,7 +159,7 @@ <h3>Japanese Text Support</h3>
<tr>
<td><code>#textEmphasis-minimal</code></td>

<td>none</td>
<td>Only <code>outside</code> position semantics are required.</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is ambiguous: it could mean

  1. only position semantics are required, and only the value outside needs to be supported, OR
  2. of the position semantics, only the value outside needs to be supported, but emphasis-style does also need to be supported, barring quoted strings as excluded by #textEmphasis-minimal

Please could you clarify this @palemieux ?

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 would say (2).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will correct to of position semantics, only is supported

Copy link
Contributor

Choose a reason for hiding this comment

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

@palemieux OK, looking forward to reviewing the change.

@palemieux
Copy link
Contributor Author

@nigelmegitt For your review

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!

@palemieux palemieux merged commit 54d55af into master Apr 26, 2018
@palemieux palemieux deleted the issue-15-limit-text-emphasis-position-to-outside branch April 11, 2019 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants