Title: [252879] trunk
- Revision
- 252879
- Author
- [email protected]
- Date
- 2019-11-26 02:11:55 -0800 (Tue, 26 Nov 2019)
Log Message
[Web Animations] Layout of children of element with forwards-filling opacity animation may be incorrect after removal
https://bugs.webkit.org/show_bug.cgi?id=204602
<rdar://problem/45311541>
Reviewed by Antti Koivisto.
Source/WebCore:
Test: webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards.html
In the case where we animate a property that affects whether an element establishes a stacking context, for instance `opacity`, the animation code,
specifically KeyframeEffect::apply(), forces a stacking context by setting setUsedZIndex(0) on the animated RenderStyle in case the element otherwise
has an "auto" z-index. This is required by the Web Animations specification (https://w3c.github.io/web-animations/#side-effects-section).
This means that a fill-forwards animation will still force the element to establish a stacking context after the animation is no longer "active". When
we remove such an animation, it may go from having a z-index to not having one, unless it had an explicit z-index provided through style. When this change
happens, RenderStyle::diff() will merely return "RepaintLayer" and thus will not foce a layout. However, updating the positions of child layers may be
necessary as the animation being removed may mean that there may not be a RenderLayer associated with that element's renderer anymore, and if that RenderLayer
had a position, then the position of child layers will no longer be correct.
Now, in the case where we destroy a layer in RenderLayerModelObject::styleDidChange(), we check whether the layer had a position before it is removed, and
update the position of child layers if it did.
* rendering/RenderLayerModelObject.cpp:
(WebCore::RenderLayerModelObject::styleDidChange):
LayoutTests:
Add a new ref test that checks that removing a forwards-filling animation that triggers a stacking context (for instance, animating opacity)
after it has completed from an element affecting layout yields the correct layout.
* webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards-expected.html: Added.
* webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (252878 => 252879)
--- trunk/LayoutTests/ChangeLog 2019-11-26 08:30:42 UTC (rev 252878)
+++ trunk/LayoutTests/ChangeLog 2019-11-26 10:11:55 UTC (rev 252879)
@@ -1,3 +1,17 @@
+2019-11-26 Antoine Quint <[email protected]>
+
+ [Web Animations] Layout of children of element with forwards-filling opacity animation may be incorrect after removal
+ https://bugs.webkit.org/show_bug.cgi?id=204602
+ <rdar://problem/45311541>
+
+ Reviewed by Antti Koivisto.
+
+ Add a new ref test that checks that removing a forwards-filling animation that triggers a stacking context (for instance, animating opacity)
+ after it has completed from an element affecting layout yields the correct layout.
+
+ * webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards-expected.html: Added.
+ * webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards.html: Added.
+
2019-11-25 Fujii Hironori <[email protected]>
[Win] Update KeyboardEvent as per the latest specification
Added: trunk/LayoutTests/webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards-expected.html (0 => 252879)
--- trunk/LayoutTests/webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards-expected.html (rev 0)
+++ trunk/LayoutTests/webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards-expected.html 2019-11-26 10:11:55 UTC (rev 252879)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+
+ .container {
+ margin-left: 100px;
+ margin-top: 100px;
+ }
+
+ .child {
+ position: relative;
+ width: 100px;
+ height: 100px;
+ background-color: black;
+ }
+
+ </style>
+</head>
+<body>
+ <div class="container">
+ <div class="child"></div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards.html (0 => 252879)
--- trunk/LayoutTests/webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards.html (rev 0)
+++ trunk/LayoutTests/webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards.html 2019-11-26 10:11:55 UTC (rev 252879)
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+
+ .container {
+ margin-left: 100px;
+ margin-top: 100px;
+ }
+
+ .animates {
+ animation: fade 1ms forwards;
+ }
+
+ .child {
+ position: relative;
+ width: 100px;
+ height: 100px;
+ background-color: black;
+ }
+
+ @keyframes fade {
+ from { opacity: 0; }
+ to { opacity: 1; }
+ }
+
+ </style>
+</head>
+<body>
+ <div class="container animates">
+ <div class="child"></div>
+ </div>
+ <script>
+
+if (window.testRunner)
+ testRunner.waitUntilDone();
+
+document.querySelector(".container").addEventListener("animationend", event => {
+ event.target.classList.remove("animates");
+ if (window.testRunner)
+ testRunner.notifyDone();
+});
+
+ </script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (252878 => 252879)
--- trunk/Source/WebCore/ChangeLog 2019-11-26 08:30:42 UTC (rev 252878)
+++ trunk/Source/WebCore/ChangeLog 2019-11-26 10:11:55 UTC (rev 252879)
@@ -1,3 +1,29 @@
+2019-11-26 Antoine Quint <[email protected]>
+
+ [Web Animations] Layout of children of element with forwards-filling opacity animation may be incorrect after removal
+ https://bugs.webkit.org/show_bug.cgi?id=204602
+ <rdar://problem/45311541>
+
+ Reviewed by Antti Koivisto.
+
+ Test: webanimations/child-layer-position-after-removal-of-animation-triggering-stacking-context-with-fill-forwards.html
+
+ In the case where we animate a property that affects whether an element establishes a stacking context, for instance `opacity`, the animation code,
+ specifically KeyframeEffect::apply(), forces a stacking context by setting setUsedZIndex(0) on the animated RenderStyle in case the element otherwise
+ has an "auto" z-index. This is required by the Web Animations specification (https://w3c.github.io/web-animations/#side-effects-section).
+
+ This means that a fill-forwards animation will still force the element to establish a stacking context after the animation is no longer "active". When
+ we remove such an animation, it may go from having a z-index to not having one, unless it had an explicit z-index provided through style. When this change
+ happens, RenderStyle::diff() will merely return "RepaintLayer" and thus will not foce a layout. However, updating the positions of child layers may be
+ necessary as the animation being removed may mean that there may not be a RenderLayer associated with that element's renderer anymore, and if that RenderLayer
+ had a position, then the position of child layers will no longer be correct.
+
+ Now, in the case where we destroy a layer in RenderLayerModelObject::styleDidChange(), we check whether the layer had a position before it is removed, and
+ update the position of child layers if it did.
+
+ * rendering/RenderLayerModelObject.cpp:
+ (WebCore::RenderLayerModelObject::styleDidChange):
+
2019-11-26 youenn fablet <[email protected]>
Queuing a task in EventLoop is not working with UserMediaRequest allow completion handler
Modified: trunk/Source/WebCore/rendering/RenderLayerModelObject.cpp (252878 => 252879)
--- trunk/Source/WebCore/rendering/RenderLayerModelObject.cpp 2019-11-26 08:30:42 UTC (rev 252878)
+++ trunk/Source/WebCore/rendering/RenderLayerModelObject.cpp 2019-11-26 10:11:55 UTC (rev 252879)
@@ -184,7 +184,12 @@
// Repaint the about to be destroyed self-painting layer when style change also triggers repaint.
if (layer()->isSelfPaintingLayer() && layer()->repaintStatus() == NeedsFullRepaint && hasRepaintLayoutRects())
repaintUsingContainer(containerForRepaint(), repaintLayoutRects().m_repaintRect);
+ // If the layer we're about to destroy had a position, then the positions of the current children will no longer be correct.
+ auto* parentLayer = layer()->parent();
+ bool layerHadLocation = !layer()->location().isZero();
layer()->removeOnlyThisLayer(); // calls destroyLayer() which clears m_layer
+ if (layerHadLocation && parentLayer && !parentLayer->renderer().needsLayout())
+ parentLayer->updateLayerPositionsAfterStyleChange();
if (s_wasFloating && isFloating())
setChildNeedsLayout();
if (s_hadTransform)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
