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

Issue 713674 link

Starred by 11 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Mac
Pri: 1
Type: Bug-Regression


Show other hotlists

Hotlists containing this issue:
Stylimations-OKR-2017-Q2


Sign in to add a comment

There is an animation jerk when animating box-shadow

Reported by pratik_b...@gridle.io, Apr 20 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36

Steps to reproduce the problem:
1. Go to this pen "https://codepen.io/anon/pen/gWrbZg"
2. Hover over the card

What is the expected behavior?
Animation should work smoothly.

What went wrong?
Animation is jerking.

Did this work before? Yes 57

Does this work in other browsers? Yes

Chrome version: 58.0.3029.81  Channel: stable
OS Version: Ubuntu 17.04
Flash Version: 

It is introduced in version 58. Before version 58 it was working fine.
 
Cc: ligim...@chromium.org
Components: Blink>Animation
Labels: Prestable-58.0.3029.81 Needs-Triage-M58 Needs-Bisect
Status: Available (was: Unconfirmed)
I suspect this is in the way we interpolate different numbers of box-shadow values. During the interpolation it grows to the lowest common multiple (in this case 6) so there would be 6 shadows stacked on top of each other with blurring making it much darker than 2 or 3.

I'll be interested in which patch the bisect points out for this.
Cc: paulir...@chromium.org
Labels: -Pri-2 -Needs-Bisect -Needs-Triage-M58 ReleaseBlock-Stable M-58 has-bisect-per-revision OS-Mac OS-Windows Pri-1
Owner: alancutter@chromium.org
Status: Assigned (was: Available)
Able to reproduce the issue on windows 7, Ubuntu 14.04 and Mac 10.12.4 using chrome version 58.0.3029.81 and canary 60.0.3073.0.
This is regression issue broken in M58.Please find the bisect information as below

Narrow bisect::
Good :: 58.0.3018.0  --   (build revision 451537)
Bad:: 58.0.3019.0   --   (build revision 451676)

Change Log::
https://chromium.googlesource.com/chromium/src/+log/2090c9f72c7eebd76726965cfc7d15cb8d1cc12d..6b6a6c64cdccdb95c20fa6c704cd44d5685f9be9

Possible suspect::
https://chromium.googlesource.com/chromium/src/+/6b6a6c64cdccdb95c20fa6c704cd44d5685f9be9

alancutter @ Could you please look into this issue if it is related to your change,else please help us in finding the appropriate owner for this issue.

Thanks,
Labels: Needs-triage-Mobile
Labels: OS-Android
Guessing this is applicable for Android also, but needs triage.

alancutter @ Please take a look at the culprit, also confirm whether this bug blocks Stable auto update.



Comment 7 by shend@chromium.org, Apr 23 2017

Labels: Update-Weekly
The old transition code was using AnimatableShadow:
https://chromium.googlesource.com/chromium/src/+/ce6b6a0d684b1ba63accead22156e32d53d169cc/third_party/WebKit/Source/core/animation/animatable/AnimatableShadow.cpp

That in turn used ShadowList::Blend:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/style/ShadowList.cpp?type=cs&q=shadowlist+blend&sq=package:chromium&l=52

Looks like the new code path for shadow interpolation was not following the spec correctly:
https://www.w3.org/TR/css3-transitions/#animatable-types
"shadow list: Each shadow in the list is interpolated via the color (as color) component, and x, y, blur, and (when appropriate) spread (as length) components. For each shadow, if one input shadow is ‘inset’ and the other is not, then the result for that shadow matches the inputs; otherwise the entire list is not interpolable. If the lists of shadows have different lengths, then the shorter list is padded at the end with shadows whose color is ‘transparent’, all lengths are ‘0’, and whose ‘inset’ (or not) matches the longer list."
Instead it treats them like repeatable lists which expand to the lowest common multiple.
We are planning a stable release soon and RC cut Monday April 24, 5.00 PM PST. alancutter@ do you think this a blocker for M58? Can it wait until M59 hits stable? Please confirm. Thanks.
Labels: -ReleaseBlock-Stable ReleaseBlock-Beta
I don't think this needs to block that release. The scenario is relatively corner casey and I haven't seen reports about it other than this one.

Comment 11 by a...@stripe.com, Apr 24 2017

It doesn't need to block the release, but this affects a production site for Stripe and we're watching the issue closely (rather than reporting again). Thanks!
#11: Thanks for letting us know, it's important for us to understand the impact of this issue.

FYI a workaround for the bug is to ensure that the box-shadows undergoing the transition have the same number of (comma separated) shadows in them. The shorter one can be padded to the same length with a no-op "0 0 0 0 transparent" shadow.
Here's the workaround applied to the original test case: https://codepen.io/anon/pen/XRjbVG
Labels: M-59
Labels: -ReleaseBlock-Beta -M-58 ReleaseBlock-Stable
For any external folks, note that when we state this won't block the M58 release, this means we'll roll the affected version out to 100% of users shortly and won't deploy the fix until M59 (currently slated to roll out ~early June).  I'll assume you have workarounds in place or this isn't a major regression for you if that's the case; as alancutter@ explained we really appreciate your input.

Since as of right now this won't block M58 stable, we shouldn't block M59 beta on it either.  Adjusting tags so that this is marked as blocking M59's stable release so this is fixed before then, though.
Tested this issue on Android 7.0.99; Build/MRA20 using chrome version #58.0.3029.83 by following steps mentioned in comment #0.

Able to reproduce the issue, Observed the jerking on hovering over the card.

Thanks,

Labels: Regressed-58
Labels: Hotlist-Google
This is breaking Google Keep in M-58.
Project Member

Comment 18 by bugdroid1@chromium.org, May 9 2017

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

commit 4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8
Author: alancutter <alancutter@chromium.org>
Date: Tue May 09 04:29:17 2017

Fix behaviour of shadow interpolation with mismatched list lengths

For shadow properties like box-shadow and text-shadow we were
incorrectly interpolating shadow lists with mismatched lengths.

The old behaviour would repeat each list to the lowest common multiple,
the correct behaviour is to pad the shorted list with "zero" shadows.

Spec: https://www.w3.org/TR/css3-transitions/#animatable-types

This change adds the "padding" behaviour to list interpolation and
composition and updates our shadow animation test cases to match
spec behaviour.

BUG= 713674 

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

[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/animations/composition/box-shadow-composition.html
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/animations/composition/text-shadow-composition.html
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/animations/interpolation/box-shadow-interpolation.html
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/animations/interpolation/text-shadow-interpolation.html
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/animations/responsive/box-shadow-responsive.html
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/transitions/multiple-shadow-transitions-expected.txt
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/transitions/multiple-shadow-transitions.html
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/CSSImageListInterpolationType.cpp
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/CSSLengthListInterpolationType.cpp
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/CSSShadowListInterpolationType.cpp
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/CSSSizeListInterpolationType.cpp
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/ListInterpolationFunctions.cpp
[modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/ListInterpolationFunctions.h

Google Keep has implemented the workaround for this bug.

Will request merge to Beta 59 once the fix has baked in Canary for a bit.
Labels: Merge-Request-59
Verified that this is fixed in Mac Canary, requesting merge to Beta.
Project Member

Comment 21 by sheriffbot@chromium.org, May 11 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 22 by bugdroid1@chromium.org, May 11 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6

commit ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6
Author: Alan Cutter <alancutter@chromium.org>
Date: Thu May 11 01:49:21 2017

Fix behaviour of shadow interpolation with mismatched list lengths

For shadow properties like box-shadow and text-shadow we were
incorrectly interpolating shadow lists with mismatched lengths.

The old behaviour would repeat each list to the lowest common multiple,
the correct behaviour is to pad the shorted list with "zero" shadows.

Spec: https://www.w3.org/TR/css3-transitions/#animatable-types

This change adds the "padding" behaviour to list interpolation and
composition and updates our shadow animation test cases to match
spec behaviour.

BUG= 713674 

Review-Url: https://codereview.chromium.org/2844213002
Cr-Commit-Position: refs/heads/master@{#470205}
(cherry picked from commit 4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8)

Review-Url: https://codereview.chromium.org/2876723002 .
Cr-Commit-Position: refs/branch-heads/3071@{#506}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/animations/composition/box-shadow-composition.html
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/animations/composition/text-shadow-composition.html
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/animations/interpolation/box-shadow-interpolation.html
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/animations/interpolation/text-shadow-interpolation.html
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/animations/responsive/box-shadow-responsive.html
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/transitions/multiple-shadow-transitions-expected.txt
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/transitions/multiple-shadow-transitions.html
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/CSSImageListInterpolationType.cpp
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/CSSLengthListInterpolationType.cpp
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/CSSShadowListInterpolationType.cpp
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/CSSSizeListInterpolationType.cpp
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/ListInterpolationFunctions.cpp
[modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/ListInterpolationFunctions.h

Status: Fixed (was: Assigned)
 Issue 722102  has been merged into this issue.
Cc: ranjitkan@chromium.org
Labels: TE-Verified-M59 TE-Verified-59.0.3071.61
Rechecked this issue on chrome version 59.0.3071.61 on Windows 10, MAC 10.12.4, Ubuntu 14.04. Fix is working as intended. On mouse hover, animation appears to be smooth.

Adding TE-verified labels.

Thanks.!

Sign in to add a comment