Diff
Modified: branches/safari-613-branch/LayoutTests/imported/w3c/ChangeLog (291666 => 291667)
--- branches/safari-613-branch/LayoutTests/imported/w3c/ChangeLog 2022-03-22 17:56:43 UTC (rev 291666)
+++ branches/safari-613-branch/LayoutTests/imported/w3c/ChangeLog 2022-03-22 17:56:48 UTC (rev 291667)
@@ -1,3 +1,77 @@
+2022-03-21 Alan Coon <alanc...@apple.com>
+
+ Cherry-pick r291282. rdar://problem/88857731
+
+ Dialog element only animates once
+ https://bugs.webkit.org/show_bug.cgi?id=236274
+
+ Reviewed by Dean Jackson, Tim Nguyen and Antti Koivisto.
+
+ LayoutTests/imported/w3c:
+
+ Import relevant WPT tests that had already been upstreamed in a previous, reverted
+ version of this patch.
+
+ * web-platform-tests/css/css-animations/dialog-animation-expected.txt: Added.
+ * web-platform-tests/css/css-animations/dialog-animation.html: Added.
+ * web-platform-tests/css/css-animations/dialog-backdrop-animation-expected.txt: Added.
+ * web-platform-tests/css/css-animations/dialog-backdrop-animation.html: Added.
+ * web-platform-tests/css/css-animations/support/testcommon.js:
+ (addElement):
+ (addDiv):
+
+ Source/WebCore:
+
+ Two issues related to CSS Animation surfaced in this bug which animates both <dialog>
+ and its ::backdrop as the dialog is open and eventually re-opened.
+
+ The first issue was that we didn't clear all CSS Animations state when a <dialog> was
+ closed and its style was set to `display: none`. We now call setAnimationsCreatedByMarkup
+ to correctly clear such state both when we identify a Styleable is newly getting
+ `display: none`. We do the same when cancelDeclarativeAnimations() is called, but also
+ call setCSSAnimationList() on the associated effect stack since that wasn't done either.
+ Now both functions do similar cleanup.
+
+ This allows us to remove removeCSSAnimationCreatedByMarkup() which did a fair bit of work
+ to clear CSS Animation state per-animation when we only ever used that function for
+ _all_ animations.
+
+ The second issue was that we never called cancelDeclarativeAnimations() for ::backdrop.
+ We now do that inside of Element::removeFromTopLayer() at a point where the code in
+ Styleable::fromRenderer() will still work as the element will still be contained in
+ Document::topLayerElements().
+
+ Tests: imported/w3c/web-platform-tests/css/css-animations/dialog-animation.html
+ imported/w3c/web-platform-tests/css/css-animations/dialog-backdrop-animation.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::removeFromTopLayer):
+ * style/Styleable.cpp:
+ (WebCore::Styleable::cancelDeclarativeAnimations const):
+ (WebCore::Styleable::updateCSSAnimations const):
+ (WebCore::removeCSSAnimationCreatedByMarkup): Deleted.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@291282 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-03-15 Antoine Quint <grao...@webkit.org>
+
+ Dialog element only animates once
+ https://bugs.webkit.org/show_bug.cgi?id=236274
+
+ Reviewed by Dean Jackson, Tim Nguyen and Antti Koivisto.
+
+ Import relevant WPT tests that had already been upstreamed in a previous, reverted
+ version of this patch.
+
+ * web-platform-tests/css/css-animations/dialog-animation-expected.txt: Added.
+ * web-platform-tests/css/css-animations/dialog-animation.html: Added.
+ * web-platform-tests/css/css-animations/dialog-backdrop-animation-expected.txt: Added.
+ * web-platform-tests/css/css-animations/dialog-backdrop-animation.html: Added.
+ * web-platform-tests/css/css-animations/support/testcommon.js:
+ (addElement):
+ (addDiv):
+
2022-03-09 Russell Epstein <repst...@apple.com>
Cherry-pick r290556. rdar://problem/89525864
Added: branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-animation-expected.txt (0 => 291667)
--- branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-animation-expected.txt (rev 0)
+++ branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-animation-expected.txt 2022-03-22 17:56:48 UTC (rev 291667)
@@ -0,0 +1,3 @@
+
+PASS CSS Animations tied to <dialog open> are canceled and restarted as the dialog is hidden and shown
+
Added: branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-animation.html (0 => 291667)
--- branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-animation.html (rev 0)
+++ branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-animation.html 2022-03-22 17:56:48 UTC (rev 291667)
@@ -0,0 +1,44 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>CSS Animations on a <dialog></title>
+<link rel="help" href=""
+<script src=""
+<script src=""
+<script src=""
+<style>
+
+dialog[open] {
+ animation: dialog-open-animation 1ms;
+}
+
+@keyframes dialog-open-animation {
+ from { opacity: 0 }
+}
+
+</style>
+<div id="log"></div>
+<script>
+
+"use strict";
+
+promise_test(async t => {
+ const dialog = addElement(t, "dialog");
+
+ // Open the dialog a first time, this should trigger a CSS Animation.
+ dialog.open = true;
+ const animations = dialog.getAnimations();
+ assert_equals(animations.length, 1, "As the <dialog> is shown intially an animation is started");
+
+ await animations[0].finished;
+
+ await waitForNextFrame();
+
+ dialog.open = false;
+ assert_equals(dialog.getAnimations().length, 0, "As the <dialog> is closed the animation is removed");
+
+ await waitForNextFrame();
+
+ dialog.open = true;
+ assert_equals(dialog.getAnimations().length, 1, "As the <dialog> is shown again an animation is started again");
+}, "CSS Animations tied to <dialog open> are canceled and restarted as the dialog is hidden and shown");
+</script>
Added: branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-backdrop-animation-expected.txt (0 => 291667)
--- branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-backdrop-animation-expected.txt (rev 0)
+++ branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-backdrop-animation-expected.txt 2022-03-22 17:56:48 UTC (rev 291667)
@@ -0,0 +1,3 @@
+
+PASS CSS Animations on a <dialog> ::backdrop are canceled and restarted as the dialog is hidden and shown
+
Added: branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-backdrop-animation.html (0 => 291667)
--- branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-backdrop-animation.html (rev 0)
+++ branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/dialog-backdrop-animation.html 2022-03-22 17:56:48 UTC (rev 291667)
@@ -0,0 +1,44 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>CSS Animations on a <dialog> ::backdrop</title>
+<link rel="help" href=""
+<script src=""
+<script src=""
+<script src=""
+<style>
+
+dialog[open]::backdrop {
+ animation: dialog-backdrop-animation 1ms;
+}
+
+@keyframes dialog-backdrop-animation {
+ from { opacity: 0 }
+}
+
+</style>
+<div id="log"></div>
+<script>
+
+"use strict";
+
+promise_test(async t => {
+ const dialog = addElement(t, "dialog");
+
+ // Open the dialog a first time, this should trigger a CSS Animation.
+ dialog.showModal();
+ const animations = dialog.getAnimations({ subtree: true });
+ assert_equals(animations.length, 1, "As the <dialog> is shown intially an animation is started on its ::backdrop");
+
+ await animations[0].finished;
+
+ await waitForNextFrame();
+
+ dialog.close();
+ assert_equals(dialog.getAnimations({ subtree: true }).length, 0, "As the <dialog> is closed the animation is removed from its ::backdrop");
+
+ await waitForNextFrame();
+
+ dialog.showModal();
+ assert_equals(dialog.getAnimations({ subtree: true }).length, 1, "As the <dialog> is shown again an animation is started again on its ::backdrop");
+}, "CSS Animations on a <dialog> ::backdrop are canceled and restarted as the dialog is hidden and shown");
+</script>
Modified: branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/support/testcommon.js (291666 => 291667)
--- branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/support/testcommon.js 2022-03-22 17:56:43 UTC (rev 291666)
+++ branches/safari-613-branch/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/support/testcommon.js 2022-03-22 17:56:48 UTC (rev 291667)
@@ -87,34 +87,46 @@
}
/**
- * Appends a div to the document body.
+ * Appends an element to the document body.
*
* @param t The testharness.js Test object. If provided, this will be used
* to register a cleanup callback to remove the div when the test
* finishes.
*
+ * @param name A string specifying the element name.
+ *
* @param attrs A dictionary object with attribute names and values to set on
* the div.
*/
-function addDiv(t, attrs) {
- var div = document.createElement('div');
+function addElement(t, name, attrs) {
+ var element = document.createElement(name);
if (attrs) {
for (var attrName in attrs) {
- div.setAttribute(attrName, attrs[attrName]);
+ element.setAttribute(attrName, attrs[attrName]);
}
}
- document.body.appendChild(div);
+ document.body.appendChild(element);
if (t && typeof t.add_cleanup === 'function') {
- t.add_cleanup(function() {
- if (div.parentNode) {
- div.remove();
- }
- });
+ t.add_cleanup(() => element.remove());
}
- return div;
+ return element;
}
/**
+ * Appends a div to the document body.
+ *
+ * @param t The testharness.js Test object. If provided, this will be used
+ * to register a cleanup callback to remove the div when the test
+ * finishes.
+ *
+ * @param attrs A dictionary object with attribute names and values to set on
+ * the div.
+ */
+function addDiv(t, attrs) {
+ return addElement(t, "div", attrs);
+}
+
+/**
* Appends a style div to the document head.
*
* @param t The testharness.js Test object. If provided, this will be used
Modified: branches/safari-613-branch/Source/WebCore/ChangeLog (291666 => 291667)
--- branches/safari-613-branch/Source/WebCore/ChangeLog 2022-03-22 17:56:43 UTC (rev 291666)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog 2022-03-22 17:56:48 UTC (rev 291667)
@@ -1,5 +1,98 @@
2022-03-21 Alan Coon <alanc...@apple.com>
+ Cherry-pick r291282. rdar://problem/88857731
+
+ Dialog element only animates once
+ https://bugs.webkit.org/show_bug.cgi?id=236274
+
+ Reviewed by Dean Jackson, Tim Nguyen and Antti Koivisto.
+
+ LayoutTests/imported/w3c:
+
+ Import relevant WPT tests that had already been upstreamed in a previous, reverted
+ version of this patch.
+
+ * web-platform-tests/css/css-animations/dialog-animation-expected.txt: Added.
+ * web-platform-tests/css/css-animations/dialog-animation.html: Added.
+ * web-platform-tests/css/css-animations/dialog-backdrop-animation-expected.txt: Added.
+ * web-platform-tests/css/css-animations/dialog-backdrop-animation.html: Added.
+ * web-platform-tests/css/css-animations/support/testcommon.js:
+ (addElement):
+ (addDiv):
+
+ Source/WebCore:
+
+ Two issues related to CSS Animation surfaced in this bug which animates both <dialog>
+ and its ::backdrop as the dialog is open and eventually re-opened.
+
+ The first issue was that we didn't clear all CSS Animations state when a <dialog> was
+ closed and its style was set to `display: none`. We now call setAnimationsCreatedByMarkup
+ to correctly clear such state both when we identify a Styleable is newly getting
+ `display: none`. We do the same when cancelDeclarativeAnimations() is called, but also
+ call setCSSAnimationList() on the associated effect stack since that wasn't done either.
+ Now both functions do similar cleanup.
+
+ This allows us to remove removeCSSAnimationCreatedByMarkup() which did a fair bit of work
+ to clear CSS Animation state per-animation when we only ever used that function for
+ _all_ animations.
+
+ The second issue was that we never called cancelDeclarativeAnimations() for ::backdrop.
+ We now do that inside of Element::removeFromTopLayer() at a point where the code in
+ Styleable::fromRenderer() will still work as the element will still be contained in
+ Document::topLayerElements().
+
+ Tests: imported/w3c/web-platform-tests/css/css-animations/dialog-animation.html
+ imported/w3c/web-platform-tests/css/css-animations/dialog-backdrop-animation.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::removeFromTopLayer):
+ * style/Styleable.cpp:
+ (WebCore::Styleable::cancelDeclarativeAnimations const):
+ (WebCore::Styleable::updateCSSAnimations const):
+ (WebCore::removeCSSAnimationCreatedByMarkup): Deleted.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@291282 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-03-15 Antoine Quint <grao...@webkit.org>
+
+ Dialog element only animates once
+ https://bugs.webkit.org/show_bug.cgi?id=236274
+
+ Reviewed by Dean Jackson, Tim Nguyen and Antti Koivisto.
+
+ Two issues related to CSS Animation surfaced in this bug which animates both <dialog>
+ and its ::backdrop as the dialog is open and eventually re-opened.
+
+ The first issue was that we didn't clear all CSS Animations state when a <dialog> was
+ closed and its style was set to `display: none`. We now call setAnimationsCreatedByMarkup
+ to correctly clear such state both when we identify a Styleable is newly getting
+ `display: none`. We do the same when cancelDeclarativeAnimations() is called, but also
+ call setCSSAnimationList() on the associated effect stack since that wasn't done either.
+ Now both functions do similar cleanup.
+
+
+ This allows us to remove removeCSSAnimationCreatedByMarkup() which did a fair bit of work
+ to clear CSS Animation state per-animation when we only ever used that function for
+ _all_ animations.
+
+ The second issue was that we never called cancelDeclarativeAnimations() for ::backdrop.
+ We now do that inside of Element::removeFromTopLayer() at a point where the code in
+ Styleable::fromRenderer() will still work as the element will still be contained in
+ Document::topLayerElements().
+
+ Tests: imported/w3c/web-platform-tests/css/css-animations/dialog-animation.html
+ imported/w3c/web-platform-tests/css/css-animations/dialog-backdrop-animation.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::removeFromTopLayer):
+ * style/Styleable.cpp:
+ (WebCore::Styleable::cancelDeclarativeAnimations const):
+ (WebCore::Styleable::updateCSSAnimations const):
+ (WebCore::removeCSSAnimationCreatedByMarkup): Deleted.
+
+2022-03-21 Alan Coon <alanc...@apple.com>
+
Cherry-pick r291175. rdar://problem/88549631
Fix WebContent jetsam that occurs when selecting text in a large e-mail
Modified: branches/safari-613-branch/Source/WebCore/dom/Element.cpp (291666 => 291667)
--- branches/safari-613-branch/Source/WebCore/dom/Element.cpp 2022-03-22 17:56:43 UTC (rev 291666)
+++ branches/safari-613-branch/Source/WebCore/dom/Element.cpp 2022-03-22 17:56:48 UTC (rev 291667)
@@ -3435,6 +3435,16 @@
layer.establishesTopLayerWillChange();
});
+ // We need to call Styleable::fromRenderer() while this element is still contained in
+ // Document::topLayerElements(), since Styleable::fromRenderer() relies on this to
+ // find the backdrop's associated element.
+ if (auto* renderer = this->renderer()) {
+ if (auto backdrop = renderer->backdropRenderer()) {
+ if (auto styleable = Styleable::fromRenderer(*backdrop))
+ styleable->cancelDeclarativeAnimations();
+ }
+ }
+
document().removeTopLayerElement(*this);
clearNodeFlag(NodeFlag::IsInTopLayer);
Modified: branches/safari-613-branch/Source/WebCore/style/Styleable.cpp (291666 => 291667)
--- branches/safari-613-branch/Source/WebCore/style/Styleable.cpp 2022-03-22 17:56:43 UTC (rev 291666)
+++ branches/safari-613-branch/Source/WebCore/style/Styleable.cpp 2022-03-22 17:56:48 UTC (rev 291667)
@@ -148,33 +148,6 @@
removeDeclarativeAnimationFromListsForOwningElement(animation);
}
-static void removeCSSAnimationCreatedByMarkup(const Styleable& styleable, CSSAnimation& cssAnimation)
-{
- styleable.animationsCreatedByMarkup().remove(&cssAnimation);
-
- if (!styleable.hasKeyframeEffects())
- return;
-
- auto& keyframeEffectStack = styleable.ensureKeyframeEffectStack();
- auto* cssAnimationList = keyframeEffectStack.cssAnimationList();
- if (!cssAnimationList || cssAnimationList->isEmpty())
- return;
-
- auto& backingAnimation = cssAnimation.backingAnimation();
- for (size_t i = 0; i < cssAnimationList->size(); ++i) {
- if (cssAnimationList->animation(i) == backingAnimation) {
- // It is important we do not make a clone of the Animation references contained
- // within cssAnimationList since sorting animations in compareCSSAnimations()
- // makes pointer comparisons to distinguish between backing animations of various
- // CSSAnimation objects.
- auto newAnimationList = cssAnimationList->shallowCopy();
- newAnimationList->remove(i);
- keyframeEffectStack.setCSSAnimationList(WTFMove(newAnimationList));
- return;
- }
- }
-}
-
void Styleable::elementWasRemoved() const
{
cancelDeclarativeAnimations();
@@ -192,13 +165,15 @@
{
if (auto* animations = this->animations()) {
for (auto& animation : *animations) {
- if (is<DeclarativeAnimation>(animation)) {
- if (is<CSSAnimation>(animation))
- removeCSSAnimationCreatedByMarkup(*this, downcast<CSSAnimation>(*animation));
+ if (is<DeclarativeAnimation>(animation))
downcast<DeclarativeAnimation>(*animation).cancelFromStyle();
- }
}
}
+
+ if (auto* effectStack = keyframeEffectStack())
+ effectStack->setCSSAnimationList(nullptr);
+
+ setAnimationsCreatedByMarkup({ });
}
static bool keyframesRuleExistsForAnimation(Element& element, const Animation& animation, const String& animationName)
@@ -231,6 +206,7 @@
for (auto& cssAnimation : animationsCreatedByMarkup())
cssAnimation->cancelFromStyle();
keyframeEffectStack.setCSSAnimationList(nullptr);
+ setAnimationsCreatedByMarkup({ });
return;
}
