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

Improve animation value syntax (#383). #608

Merged
merged 2 commits into from
Feb 13, 2018

Conversation

skynavga
Copy link
Collaborator

Closes #383.

@skynavga skynavga added this to the Editor's CR Work List milestone Jan 30, 2018
@skynavga skynavga self-assigned this Jan 30, 2018
@@ -18080,28 +18080,25 @@ or ending (final) of the attribute targeted by the animation.</p>
<td>
<eg xml:space="preserve">
&lt;animation-value&gt;
: string
: ([^;] | escape)+
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a regexp? Do we use those anywhere else? Does it imply that any code point other than ; can be used? If so, it seems to admit use of characters that would otherwise fall within <lwsp> which makes processing <animation-value-list> rather tricky. I'm not convinced we can use a shortcut like this, sadly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is standard syntax. Nothing new, and not invented by TTWG. Look at quoted string for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh no! It's broken there too. From the references I've just read about character classes, the example here means "any character not a ;" and the example in quoted string ([^"\\] and similar for a single quote) both admit whitespace and control characters and are not restricted at all otherwise (apart from not permitting the quote characters that need to be escaped). <animation-value> even permits unescaped attribute delimiters, which would surely cause a validation error, or some other problems.

Since the <animation-value-list> can be used on any styling attribute, the value type for each <animation-value> in the list needs to be unambiguous in the context of the permitted values for the styling attribute on which it is specified. Styles like tts:fontFamily permit any quoted string, and look like they provide an escaping mechanism but that restriction needs to apply to <animation-value> too - it seems to be the most liberal style attribute that we need to handle.

Copy link
Collaborator Author

@skynavga skynavga Feb 7, 2018

Choose a reason for hiding this comment

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

@nigelmegitt I don't see any breakage here at all. You are correct that the syntactic expression [;\\] means "any character other than ; and \\". You are also correct that [^"\\] means "any character other than " and \\". This is by design, is part of existing implementations, and has resulted in no bug report that I know of.

The only syntactic requirement of <animation-value> from the perspective of parsing <animation-value-list> is that the former contains no unescaped ;. In other words, the parsing of <animation-value-list> consists solely of splitting a string where ; not preceded by \\ is the separator; the result of this split operation produces an array of strings, each of which must satisfy the syntax rules of the associated style property.

Copy link
Contributor

Choose a reason for hiding this comment

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

@skynavga Right, so <animation-value> is used in attributes, but can contain XML attribute delimiters and control character codes. How is that not broken?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because XML attribute processing occurs before value parsing. If it is not well formed as an XML attribute in the first place, it is already invalid and will never reach value parsing.

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.

Based on the new context that this is post-XML attribute processing, approving. That was there before but I hadn't thought about it.

@skynavga skynavga merged commit 79bb972 into master Feb 13, 2018
@skynavga skynavga removed their assignment Feb 13, 2018
@skynavga skynavga deleted the issue-0383-animation-value-syntax branch March 9, 2018 21:06
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.

None yet

3 participants