New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 601672 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Throw TypeErrors on invalid easings passed to Element.animate

Project Member Reported by suzyh@chromium.org, Apr 8 2016

Issue description

According to the Web Animations spec, invalid values for easing should result in a TypeError:
https://w3c.github.io/web-animations/#dom-animationeffecttiming-easing

While investigating implementing this, we discovered that the Web Animations polyfill had a bug that would translate a valid value for easing (e.g. 'linear') into a function ('function (a){return a}'). This bug has now been fixed in https://github.com/web-animations/web-animations-js/tree/2.2.0 but any sites continuing to use an older version of the polyfill may hit this bug.

The Blink animation team's strategy is to
- throw a TypeError on all invalid easings except for "function (a){return a}"
- add a deprecation warning for "function (a){return a}" as an easing in M52 with an aim to remove support for it in M54 (at which point, this will also cause a TypeError)
- add use counters to track how often people are encountering function-values for easings, both the specific "function (a){return a}" case and any other function


 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51a95e49695ae972bc8315dca6b4a3c08a486910

commit 51a95e49695ae972bc8315dca6b4a3c08a486910
Author: suzyh <suzyh@chromium.org>
Date: Fri Apr 08 10:21:41 2016

Add use counters for function values of easing

Due to a bug in old versions of the web-animations-next polyfill, in
some circumstances the string passed in to
AnimationInputHelpers::parseTimingFunction may be a Javascript
function instead of the allowed values from the spec
(http://w3c.github.io/web-animations/#dom-animationeffecttimingreadonly-easing)
This bug was fixed in
https://github.com/web-animations/web-animations-next/pull/423
and we want to track how often it is still being hit. The linear case
is special because 'linear' is the default value for easing.

BUG= 601672 

Review URL: https://codereview.chromium.org/1867803002

Cr-Commit-Position: refs/heads/master@{#386037}

[add] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/LayoutTests/animations/function-easing-use-counters-linear.html
[add] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/LayoutTests/animations/function-easing-use-counters-other1.html
[add] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/LayoutTests/animations/function-easing-use-counters-other2.html
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/animation/AnimationEffectTiming.cpp
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/animation/AnimationInputHelpers.cpp
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/animation/AnimationInputHelpers.h
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/animation/AnimationInputHelpersTest.cpp
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/animation/EffectInput.cpp
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/animation/ElementAnimation.h
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/animation/KeyframeEffect.cpp
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/animation/TimingInput.cpp
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/animation/TimingInput.h
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/animation/TimingInputTest.cpp
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] https://crrev.com/51a95e49695ae972bc8315dca6b4a3c08a486910/tools/metrics/histograms/histograms.xml

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/84a58b756e4c8c2746cdfc8f326c11f06d0b2865

commit 84a58b756e4c8c2746cdfc8f326c11f06d0b2865
Author: suzyh <suzyh@chromium.org>
Date: Thu Apr 14 07:03:32 2016

Throw TypeError if easing string is invalid.

Currently, if an animation is created with options containing an
invalid value for 'easing', it is ignored and the default easing is
used instead. This patch changes the implementation to throw a
TypeError as specced
(http://w3c.github.io/web-animations/#dom-animationeffecttiming-easing).

As described in the bug referenced below, we are temporarily exempting
"function (a){return a}" from throwing a TypeError. A TODO is added to
remove this exemption for M54.

This patch also adds a layout test animation-effect-timing-easing.html
that is equivalent to the unit test ParseAnimationTimingFunction
in AnimationInputHelpersTest.cpp. This enables the test results to
be compared against those of other browsers. Note that the test is
therefore duplicated because the unit test is not removed.

BUG= 601672 

Review URL: https://codereview.chromium.org/1851003002

Cr-Commit-Position: refs/heads/master@{#387250}

[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/LayoutTests/animations/function-easing-use-counters-linear.html
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/LayoutTests/animations/function-easing-use-counters-other1.html
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/LayoutTests/animations/function-easing-use-counters-other2.html
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/LayoutTests/inspector/animation/animation-timeline-expected.txt
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/LayoutTests/inspector/animation/animation-timeline.html
[add] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/LayoutTests/web-animations-api/animation-effect-timing-easing.html
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/AnimationEffectTiming.cpp
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/AnimationEffectTiming.h
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/AnimationEffectTiming.idl
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/AnimationInputHelpers.cpp
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/AnimationInputHelpers.h
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/AnimationInputHelpersTest.cpp
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/EffectInput.cpp
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/ElementAnimation.h
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/KeyframeEffect.cpp
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/KeyframeEffectTest.cpp
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/TimingInput.cpp
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/TimingInput.h
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/animation/TimingInputTest.cpp
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/third_party/WebKit/Source/core/frame/Deprecation.cpp
[modify] https://crrev.com/84a58b756e4c8c2746cdfc8f326c11f06d0b2865/ui/file_manager/file_manager/foreground/elements/files_ripple.js

Comment 3 by suzyh@chromium.org, Apr 15 2016

Current status: The UseCounters added in #c1 were included in M51. The TypeError + deprecation warning patch in #c2 will be in M52.

The next thing to do will be: between M53 and M54 branch points (July-Aug 2016), review the UseCounter values and ideally remove the exemption for "function (a){return a}". There shouldn't be anything further required on this bug until then.

Comment 4 by suzyh@chromium.org, Apr 20 2016

An update from the polyfill:

This bug that was fixed in https://github.com/web-animations/web-animations-js/tree/2.2.0 has been cherrypicked on top of earlier releases, to make it easier for sites to incorporate the fix without a major upgrade.

The versions of the polyfill that contain the fix are now:
https://github.com/web-animations/web-animations-js/tree/2.2.0
https://github.com/web-animations/web-animations-js/tree/2.1.5
https://github.com/web-animations/web-animations-js/tree/2.0.1
https://github.com/web-animations/web-animations-js/tree/1.0.8

Comment 5 by suzyh@chromium.org, May 4 2016

Status: ExternalDependency (was: Started)
Changing the status to ExternalDependency since this is waiting for time to pass and UseCounter information to be collected.

Comment 6 by suzyh@chromium.org, May 25 2016

Labels: Update-quarterly
This recently came up with a polymer + closure app I maintain, but the TypeError we get is "Uncaught TypeError: Failed to execute 'animate' on 'Element': 'function (a) { return a; }' is not a valid value for easing", which seems to not be matched by the filter included here. Would it be possible to make the filter a bit looser?

Comment 8 by phistuck@gmail.com, Jun 6 2016

#7 -
Can you update the web animation polyfill instead? The fix has been cherry-picked to older versions as well.
Obviously that should happen too, but if we're still in data gathering stages for the exception, a looser filter is probably prudent. Depending on JS tooling, the identity function may stringify slightly differently and there could be other affected sites.
achernya/davidben: Thanks for the report and info. I'll look into it and update this bug with the outcome.
The UseCounters have been in stable for about a week now. For the strict match linear specified above ("function (a){return a}") we're seeing less than 0.002% incidence. All other function easings, including linear functions stringified differently, are at approximately 0%.
Linear: https://www.chromestatus.com/metrics/feature/timeline/popularity/1295
Other: https://www.chromestatus.com/metrics/feature/timeline/popularity/1296

Given that the linear case will also start causing TypeErrors in a couple of months (and so developers will need to update the Web Animations polyfill soon regardless), and the incidence of differently-stringified-linear functions is so low, we are not planning to implement looser matching for the linear easing. Instead, please update to a newer version of the Web Animations polyfill (directly or via an update to Polymer) as listed in #4.

Let me know if you need any further info.
Labels: -Update-Quarterly Update-Fortnightly
Status: Started (was: ExternalDependency)

Comment 13 by suzyh@chromium.org, Jul 20 2016

Progress report: I've written the patch to do the removal https://codereview.chromium.org/2129783003 but need to dig into the usage in more detail before we proceed.
The incidence of the linear case UseCounter is dropping, but not as fast as we'd like, so we're going to hold off on removing the exemption until the next release. The removal is now expected to be in M55.

Comment 15 by suzyh@chromium.org, Aug 17 2016

In the last week, the incidence of the linear case UseCounter has dropped substantially:
https://www.chromestatus.com/metrics/feature/timeline/popularity/1295

As such, the removal is proceeding as originally planned in M54.
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c2c07865e41b3cbb10e8c77508b7f86709a7ce3

commit 5c2c07865e41b3cbb10e8c77508b7f86709a7ce3
Author: suzyh <suzyh@chromium.org>
Date: Wed Aug 17 07:01:17 2016

Remove linear-function easing TypeError exemption.

Invalid values for 'easing' should throw a TypeError
(http://w3c.github.io/web-animations/#dom-animationeffecttiming-easing).
In crrev.com/1851003002, this was implemented for all invalid values except
"function (a){return a}", as described in the bug referenced below. The
incidence of this special case is below the feature removal threshold. This
patch removes the exemption, bringing the code in line with the spec.

BUG= 601672 

Review-Url: https://codereview.chromium.org/2129783003
Cr-Commit-Position: refs/heads/master@{#412465}

[modify] https://crrev.com/5c2c07865e41b3cbb10e8c77508b7f86709a7ce3/third_party/WebKit/LayoutTests/animations/function-easing-use-counters-linear.html
[delete] https://crrev.com/7cfe9a88068e68478bf5d5a25f5f184d087ac1a9/third_party/WebKit/LayoutTests/web-animations-api/animation-effect-timing-easing-expected.txt
[modify] https://crrev.com/5c2c07865e41b3cbb10e8c77508b7f86709a7ce3/third_party/WebKit/Source/core/animation/AnimationInputHelpers.cpp
[modify] https://crrev.com/5c2c07865e41b3cbb10e8c77508b7f86709a7ce3/third_party/WebKit/Source/core/frame/Deprecation.cpp

Comment 17 by suzyh@chromium.org, Aug 17 2016

Status: Fixed (was: Started)
Project Member

Comment 18 by bugdroid1@chromium.org, Nov 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b58a5dbca30f6a3d81b648916f823d796ce6fa4

commit 9b58a5dbca30f6a3d81b648916f823d796ce6fa4
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Tue Nov 14 14:45:46 2017

Clean up kWebAnimationsEasingAsFunction{Linear,Other} UseCounters

The change these were introduced to track was over 1.5 years ago. We
collected the necessary data and the deprecation workaround was removed
a year ago, so its time for these counters to be removed.

Bug:  601672 
Change-Id: I64f0e78152068000622421111765678b6ce798d7
Reviewed-on: https://chromium-review.googlesource.com/767056
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516298}
[delete] https://crrev.com/2aac74d3581683516ed149948903875b741c125a/third_party/WebKit/LayoutTests/animations/usecounters/function-easing-use-counters-linear.html
[delete] https://crrev.com/2aac74d3581683516ed149948903875b741c125a/third_party/WebKit/LayoutTests/animations/usecounters/function-easing-use-counters-other1.html
[delete] https://crrev.com/2aac74d3581683516ed149948903875b741c125a/third_party/WebKit/LayoutTests/animations/usecounters/function-easing-use-counters-other2.html
[modify] https://crrev.com/9b58a5dbca30f6a3d81b648916f823d796ce6fa4/third_party/WebKit/Source/core/animation/AnimationEffectTiming.cpp
[modify] https://crrev.com/9b58a5dbca30f6a3d81b648916f823d796ce6fa4/third_party/WebKit/Source/core/animation/AnimationInputHelpers.cpp
[modify] https://crrev.com/9b58a5dbca30f6a3d81b648916f823d796ce6fa4/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/9b58a5dbca30f6a3d81b648916f823d796ce6fa4/tools/metrics/histograms/enums.xml

Sign in to add a comment