[css-animations] Change an AnimationEvent member from float to double #2666
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…' to 'double' As per the web-idl spec, float should only be used if there is a specific reason, as double much more closely matches the ECMAScript Number type. In the case of AnimationEvent there does not appear to be such a reason. Furthermore, switching to double allows the elapsedTime to reach higher values without overflowing. A rough calculation (with some assumptions and possible errors) suggests that this change will raise the celing from ~4.5 hours to years.
Contributor
Author
|
@birtles for review |
Contributor
|
Looks good to me. I checked that this change doesn't break any changes in Firefox's test suite and it seems fine. |
Contributor
|
@stephenmcgruer Do you mind submitting a similar PR for transitions? |
Contributor
Author
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
May 14, 2018
w3c/csswg-drafts#2666 and w3c/csswg-drafts#2671 updated the spec so that elapsedTime as a member of AnimationEvent and TransitionEvent is specified as a double instead of a float. Remove the TODOs as our implementation is now correct. Change-Id: I1c7abf14765fdbd46e37b4f8c506fed574cecdd0 Reviewed-on: https://chromium-review.googlesource.com/1056787 Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Commit-Queue: Chris Nardi <cnardi@chromium.org> Cr-Commit-Position: refs/heads/master@{#558285}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As per the web-idl spec, float should only be used if there is a specific
reason, as double much more closely matches the ECMAScript Number
type. In the case of AnimationEvent's 'elapsedTime' member there does
not appear to be such a reason.
Furthermore, switching to double allows the elapsedTime to reach higher
values without overflowing. A rough calculation (with some assumptions
and possible errors) suggests that this change will raise the celing
from ~4.5 hours to years.