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

Widen data type for tta:{gain,pan} (#955). #956

Merged
merged 2 commits into from
Aug 3, 2018

Conversation

skynavga
Copy link
Collaborator

Closes #955.

@skynavga skynavga added this to the CR3 milestone Jul 31, 2018
@skynavga skynavga self-assigned this Jul 31, 2018
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 would prefer not to do this but approving for now since it is non-normative.

The impact of not doing it is that there would be some false positive validations, potentially, where values like 0. would be accepted when they should not be, as per the issue. I don't think that's particularly a bad thing, compared to what this pull request does, which is to allow a huge amount of invalid values unless validator code does further checking.

By the way, there's a general systemic issue with all the styling namespaces attribute values that are not [xs:]string, in that they always generate false negatives when used in the context of an <animation-value-list>, because the semicolon delimiter is not permitted in the schema, though it is in the spec.

@skynavga skynavga merged commit a9ca92b into master Aug 3, 2018
@skynavga skynavga removed their assignment Aug 3, 2018
@skynavga
Copy link
Collaborator Author

skynavga commented Aug 3, 2018

Merged early per WG resolution.

@skynavga
Copy link
Collaborator Author

skynavga commented Aug 6, 2018

@nigelmegitt re: your last paragraph of #956 (review), I have just checked the spec and schemas, and all continuously animatable styles except for tts:opacity are currently defined with a type of xs:string; therefore, to avoid false negatives during schema evaluation when using tts:opacity on tt:animate, we should change the type used with tts:opacity to xs:string in the schemas.

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