New issue
Advanced search Search tips

Issue 764783 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

div with animated 'transform' should remain a containing block for positioned descendants, even if it's animating the value "transform:none"

Project Member Reported by dholb...@gmail.com, Sep 13 2017

Issue description

Chrome Version: 62.0.3202.9 (Official Build) dev (64-bit)
OS: Ubuntu 17.10 beta (but unlikely to be OS-specific)

What steps will reproduce the problem?
(1) Visit https://jsfiddle.net/kkpznn6L/

What is the expected result?
No red should be visible.

What happens instead?
Red is visible. (The fixed-pos lime div abandons its red containing block and uses the viewport as its containing block instead.)


Please use labels and text to provide additional information.
============
The animation ends at "transform:none", which normally (if there weren't any animations) would mean that the red div should stop being a containing block for positioned children, which would mean that Chrome's behavior here is correct [1].

However, that spec text doesn't matter because there's another effect that supercedes it, from the CSS Animations spec:
"While an animation [...] has an animation-fill-mode of 'forwards' or 'both', the user agent must act as if the will-change property ([css-will-change-1]) on the element additionally includes all the properties animated by the animation."
https://drafts.csswg.org/css-animations/#animations

So the rendering should be the same as what it'd be if "will-change:transform" were present.  For comparison, here is a reference case with that style manually added:
https://jsfiddle.net/kkpznn6L/1/

Chrome renders *that one* correctly (no red).

So, that means Chrome is mis-rendering the original testcase (https://jsfiddle.net/kkpznn6L/ ) -- it's not honoring this bit of CSS Animations spec text.


[1] "...any value other than 'none' for the transform also causes the element to ... [act] as a containing block for fixed positioned descendants."
https://drafts.csswg.org/css-transforms/#transform-rendering
 

Comment 1 by dholb...@gmail.com, Sep 13 2017

Firefox 55 and Edge 15 each produce the expected result, BTW.

Chrome 62 dev and Safari 10.1.2 produce the incorrect/unexpected result.

Comment 2 by dholb...@gmail.com, Sep 13 2017

WebKit version of this bug: https://bugs.webkit.org/show_bug.cgi?id=176858

Comment 3 by shend@chromium.org, Sep 13 2017

Labels: Needs-Bisect
Requesting bisect to see if this is a regression.

Comment 4 by dholb...@gmail.com, Sep 14 2017

(I suspect it's not a regression -- the animation spec text in question is relatively new, I think.)
Cc: jmukthavaram@chromium.org alancutter@chromium.org
Labels: -Type-Bug -Pri-3 -Needs-Bisect hasbisect-per-revision Needs-Triage-M63 M-63 OS-Linux OS-Mac OS-Windows Pri-1 Type-Bug-Regression
Owner: timloh@chromium.org
Status: Assigned (was: Untriaged)
Able to reproduce this issue on windows 7, Mac 10.12.6,Linux Ubuntu 14.04 with Chrome stable version-61.0.3163.100 and Canary-63.0.3235.0.

Manual Bisect:
Good Build—52.0.2717.0 - Revision-389638
Bad Build—52.0.2718.0 -Revision-389938

Bisect Tool Info:
-----------------
You are probably looking for a change made after 389692 (known good), but no later than 389696 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspectas some perf builds might get missing due to failure.
https://chromium.googlesource.com/chromium/src/+log/003408ad8ca6ce1a66bd796d13439f3795f8f19c..b721ba4c7c32b789911cdf125f4871f9b442320b

Possible suspect:
--------------
https://chromium.googlesource.com/chromium/src/+/7445af7fe2c76b998303fdbe6969a6064b741534

Review URL:
----------
https://codereview.chromium.org/1866333002

AS alancutter@ is OOO till 2018,assigning to the reviewer timloh@to take a look into this issue.

timloh@,Kindly take a look and please help us to reassign this issue to a right owner if not with respect to this change.

Thanks.!
Labels: Update-Quarterly
Owner: ----
Status: Available (was: Assigned)
Labels: Hotlist-Polish
Cc: smcgruer@chromium.org
Labels: -Pri-1 -Type-Bug-Regression Pri-2 Type-Bug
Owner: xidac...@chromium.org
Status: Assigned (was: Available)
Still broken in M66. As per #4, I don't think we should treat this as a regression. However both Firefox and Edge appear to get this case correct.

Xida - do you have the cycles to look at this currently? First step will probably to get a handle on how large a task it is; we need to figure out how to pretend that 'will-change' is specified for animating properties as long as the animation is not finished (and fill-mode is forward/both).

We should also check the behavior for Web Animations (on us, and on Firefox).
From our conversation; http://output.jsbin.com/dalani is an interesting test case. We have to make sure that immediately after setting the class list, we properly compute the containment and output a non-(0,0) position for the positioned descendant.

(This test case works in Firefox today, obviously doesn't work in Chrome at all due to this bug :)).
BTW issue 882625 is related to this. I have WIP [1] patch to update will-change to take into account "current" composited animations. Turned out will-change has more side-effects than I expected so I need to understand them more before landing this.

This however does not fix your issue as it is. I think mostly because ComputedStyle::SetHasCurrentXYZAnimation only considers *current* animations and ignores *in effect* animations [2].

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1216714
[2] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/animation/element_animations.cc?type=cs&sq=package:chromium&g=0&l=92


Project Member

Comment 11 by bugdroid1@chromium.org, Oct 25

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

commit ee27dd97add2ac408f863f49b9b7ec5bef307fc3
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Oct 25 01:07:52 2018

Add HasTransformAnimationWithForwardsOrBothFillMode in style

When a transform animation has animation-fill-mode being 'forward' or
'both', the element should be a containing block even if the animation
has finished.

This CL added HasTransformAnimationWithForwardsOrBothFillMode in
computed style. We check whether an animation is transform animation
and has 'forward' or 'both' fill-mode. If the condition is true, we
set this bit true.

Bug:  764783 
Change-Id: Ie4c4280bbeca5ce79b7fc60e299dce306283589a
Reviewed-on: https://chromium-review.googlesource.com/c/1066630
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Anders Ruud <andruud@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602554}
[modify] https://crrev.com/ee27dd97add2ac408f863f49b9b7ec5bef307fc3/third_party/blink/renderer/core/animation/compositor_animations_test.cc
[modify] https://crrev.com/ee27dd97add2ac408f863f49b9b7ec5bef307fc3/third_party/blink/renderer/core/animation/element_animations.cc
[add] https://crrev.com/ee27dd97add2ac408f863f49b9b7ec5bef307fc3/third_party/blink/renderer/core/animation/test_data/css-animation-with-fill-mode.html
[add] https://crrev.com/ee27dd97add2ac408f863f49b9b7ec5bef307fc3/third_party/blink/renderer/core/animation/test_data/web-animation-with-fill-mode.html
[modify] https://crrev.com/ee27dd97add2ac408f863f49b9b7ec5bef307fc3/third_party/blink/renderer/core/css/computed_style_extra_fields.json5
[modify] https://crrev.com/ee27dd97add2ac408f863f49b9b7ec5bef307fc3/third_party/blink/renderer/core/style/computed_style.h

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 30

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

commit 7362c167193f7721b49e019f934b5a6584e228b7
Author: Xida Chen <xidachen@chromium.org>
Date: Tue Oct 30 19:42:51 2018

[Test] Adds test of IsStackingContext for animation with fill-mode

In our previous CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1066630
We fixed the problem where an element with transform animation and
fill-mode being 'forward' or 'both' should be containing block. This
CL adds test to ensure that the style has IsStackingContext = true in
those cases.

Bug:  764783 
Change-Id: Ib110c52f3afd5f68028569c528145162b4c48ed1
Reviewed-on: https://chromium-review.googlesource.com/c/1299203
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603986}
[modify] https://crrev.com/7362c167193f7721b49e019f934b5a6584e228b7/third_party/blink/renderer/core/animation/compositor_animations_test.cc

Status: Fixed (was: Assigned)

Sign in to add a comment