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.

Sign up for GitHub

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

Jump to bottom

Removed redundant clause at 8.4.1.3 Chained Referential Styling #296

Merged
merged 3 commits into from
Jan 4, 2018
Merged

Removed redundant clause at 8.4.1.3 Chained Referential Styling #296

merged 3 commits into from
Jan 4, 2018

Conversation

palemieux
Copy link
Contributor

Closes #275

@palemieux
Removed redundant clause at '
22e38b9 
8.4.1.3 Chained Referential Styling' (#275)
@palemieux palemieux added this to the 3rd Ed milestone Dec 16, 2017
@palemieux palemieux self-assigned this Dec 16, 2017
skynavga
skynavga requested changes Dec 16, 2017
View reviewed changes
spec/ttml1.xml
@@ -6654,9 +6654,6 @@ ref="semantics-style-inheritance-content"/> below.</p>
other out-of-line style properties, thus creating a chain of references starting at the
affected element. When expressed in this manner, the association of style information
is referred to as <emph>chained referential styling</emph>.</p>
<p>If the same style property is specified in more than one referenced
Copy link
Contributor

Choose a reason for hiding this comment

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

I am opposed to removing this. If there is a problem with it, then let's fix it.

nigelmegitt
nigelmegitt previously requested changes Dec 17, 2017
View reviewed changes
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.

This is fixing the wrong thing in my opinion - it is the example that is misleading not the sentence being removed here.

@palemieux
Copy link
Contributor Author

@nigelmegitt @skynavga Please propose a fix to the example then.

@nigelmegitt
Copy link
Contributor

@palemieux @skynavga I don't want to have this discussion in two places at once - right now it seems like it is happening in #275 so I'll wait for something closer to a conclusion there before proposing something additional here. I did propose an alternative example already in that thread.

@skynavga
Copy link
Contributor

skynavga commented Jan 4, 2018

I can accept removing the second paragraph of 8.4.1.3 and adding a note under the example as follows:

Note: The text of the second paragraph in this example is yellow, since tts:color='yellow' effectively overwrites (is merged over) the tts:color='white' that style s2 obtains by a reference to style s1.

@palemieux
Copy link
Contributor Author

Rendered at https://rawgit.com/w3c/ttml1/issue-275-redundance-clause-at-8-4-1-3-build/index.html

nigelmegitt
nigelmegitt reviewed Jan 4, 2018
View reviewed changes
spec/ttml1.xml
@@ -6654,9 +6654,6 @@ ref="semantics-style-inheritance-content"/> below.</p>
other out-of-line style properties, thus creating a chain of references starting at the
affected element. When expressed in this manner, the association of style information
is referred to as <emph>chained referential styling</emph>.</p>
<p>If the same style property is specified in more than one referenced
style set, then the last referenced style set applies, where the order of application starts from
the affected element and proceeds to referenced style sets, and, in turn, to subsequent referenced style sets.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

I would still like to add a replacement sentence that explains that when chained referential styling is used each referenced style is fully resolved before the reference is followed.

nigelmegitt
nigelmegitt reviewed Jan 4, 2018
View reviewed changes
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 don't believe this fully addresses the issue since I would like a clear short sentence to be added in place of the removed paragraph to explain that each referenced style must be resolved into specified styles before being followed.

@nigelmegitt nigelmegitt dismissed their stale review January 4, 2018 16:35

Dismissing my earlier non-approval in favour of my more recent review comment

@palemieux palemieux merged commit d9e3946 into master Jan 4, 2018
@skynavga skynavga added editorial and removed agenda labels Jan 4, 2018
@skynavga skynavga deleted the issue-275-redundance-clause-at-8-4-1-3 branch March 11, 2018 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nigelmegitt nigelmegitt nigelmegitt left review comments

@skynavga skynavga skynavga approved these changes

@andreastai andreastai andreastai approved these changes

Assignees

@palemieux palemieux

Labels
editorial
Projects
None yet
Milestone
3rd Ed CR
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@palemieux @nigelmegitt @skynavga @andreastai