Title: [246170] trunk
Revision
246170
Author
s...@apple.com
Date
2019-06-06 14:56:55 -0700 (Thu, 06 Jun 2019)

Log Message

REGRESSION (r243121): Load event should not be fired while animating the 'externalResourcesRequired' attribute 
https://bugs.webkit.org/show_bug.cgi?id=198576

Reviewed by Simon Fraser.

Source/WebCore:

Firing the load event should only happen when dynamic update changes the
attribute 'externalResourcesRequired'. Animating this attribute should
not fire the load event.

When stopping the animations, applyAnimatedPropertyChange() should be
called first then stopAnimation() is called second. The target element
should know that its svgAttributeChanged() is called because of animating
the attribute. So it can differentiate this case from the dynamic update.

Test: svg/animations/animate-externalResourcesRequired-no-load-event.html

* svg/SVGExternalResourcesRequired.cpp:
(WebCore::SVGExternalResourcesRequired::svgAttributeChanged):
* svg/properties/SVGAnimatedPropertyAnimator.h:

LayoutTests:

* svg/animations/animate-externalResourcesRequired-no-load-event-expected.txt: Added.
* svg/animations/animate-externalResourcesRequired-no-load-event.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (246169 => 246170)


--- trunk/LayoutTests/ChangeLog	2019-06-06 20:23:51 UTC (rev 246169)
+++ trunk/LayoutTests/ChangeLog	2019-06-06 21:56:55 UTC (rev 246170)
@@ -1,3 +1,14 @@
+2019-06-05  Said Abou-Hallawa  <sabouhall...@apple.com>
+
+        REGRESSION (r243121): Load event should not be fired while animating the 'externalResourcesRequired' attribute 
+        https://bugs.webkit.org/show_bug.cgi?id=198576
+
+        Reviewed by Simon Fraser.
+
+        * svg/animations/animate-externalResourcesRequired-no-load-event-expected.txt: Added.
+        * svg/animations/animate-externalResourcesRequired-no-load-event.html: Added.
+
+2019-06-04  Takashi Komori  <takashi.kom...@sony.com>
 2019-06-06  Antoine Quint  <grao...@apple.com>
 
         Restrict fast clicks everywhere to desktop content mode

Added: trunk/LayoutTests/svg/animations/animate-externalResourcesRequired-no-load-event-expected.txt (0 => 246170)


--- trunk/LayoutTests/svg/animations/animate-externalResourcesRequired-no-load-event-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/animations/animate-externalResourcesRequired-no-load-event-expected.txt	2019-06-06 21:56:55 UTC (rev 246170)
@@ -0,0 +1,3 @@
+Test passes if it does not assert in debug builds.
+
+

Added: trunk/LayoutTests/svg/animations/animate-externalResourcesRequired-no-load-event.html (0 => 246170)


--- trunk/LayoutTests/svg/animations/animate-externalResourcesRequired-no-load-event.html	                        (rev 0)
+++ trunk/LayoutTests/svg/animations/animate-externalResourcesRequired-no-load-event.html	2019-06-06 21:56:55 UTC (rev 246170)
@@ -0,0 +1,24 @@
+<body>
+    <p>Test passes if it does not assert in debug builds.</p>
+    <option id="option">
+        <svg id="svg">
+            <set id="set" attributeName="externalResourcesRequired" dur="20ms" to="true"/>
+        </svg>
+    </option>
+    <select id="select">
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+
+        svg.addEventListener("load", () => {
+            select.add(option);
+        });
+
+        set.addEventListener("endEvent", () => {
+            if (window.testRunner)
+                testRunner.notifyDone();
+        });
+    </script>
+</body>

Modified: trunk/Source/WebCore/ChangeLog (246169 => 246170)


--- trunk/Source/WebCore/ChangeLog	2019-06-06 20:23:51 UTC (rev 246169)
+++ trunk/Source/WebCore/ChangeLog	2019-06-06 21:56:55 UTC (rev 246170)
@@ -1,3 +1,26 @@
+2019-06-05  Said Abou-Hallawa  <sabouhall...@apple.com>
+
+        REGRESSION (r243121): Load event should not be fired while animating the 'externalResourcesRequired' attribute 
+        https://bugs.webkit.org/show_bug.cgi?id=198576
+
+        Reviewed by Simon Fraser.
+
+        Firing the load event should only happen when dynamic update changes the
+        attribute 'externalResourcesRequired'. Animating this attribute should
+        not fire the load event.
+
+        When stopping the animations, applyAnimatedPropertyChange() should be
+        called first then stopAnimation() is called second. The target element
+        should know that its svgAttributeChanged() is called because of animating
+        the attribute. So it can differentiate this case from the dynamic update.
+
+        Test: svg/animations/animate-externalResourcesRequired-no-load-event.html
+
+        * svg/SVGExternalResourcesRequired.cpp:
+        (WebCore::SVGExternalResourcesRequired::svgAttributeChanged):
+        * svg/properties/SVGAnimatedPropertyAnimator.h:
+
+2019-06-05  Saam Barati  <sbar...@apple.com>
 2019-06-06  Zalan Bujtas  <za...@apple.com>
 
         [LFC][IFC] Move baseline and line height computation to a dedicated function

Modified: trunk/Source/WebCore/svg/SVGExternalResourcesRequired.cpp (246169 => 246170)


--- trunk/Source/WebCore/svg/SVGExternalResourcesRequired.cpp	2019-06-06 20:23:51 UTC (rev 246169)
+++ trunk/Source/WebCore/svg/SVGExternalResourcesRequired.cpp	2019-06-06 21:56:55 UTC (rev 246170)
@@ -55,7 +55,7 @@
     // Handle dynamic updates of the 'externalResourcesRequired' attribute. Only possible case: changing from 'true' to 'false'
     // causes an immediate dispatch of the SVGLoad event. If the attribute value was 'false' before inserting the script element
     // in the document, the SVGLoad event has already been dispatched.
-    if (!externalResourcesRequired() && !haveFiredLoadEvent() && !isParserInserted()) {
+    if (!externalResourcesRequiredAnimated().isAnimating() && !externalResourcesRequired() && !haveFiredLoadEvent() && !isParserInserted()) {
         setHaveFiredLoadEvent(true);
 
         ASSERT(m_contextElement.haveLoadedRequiredResources());

Modified: trunk/Source/WebCore/svg/properties/SVGAnimatedPropertyAnimator.h (246169 => 246170)


--- trunk/Source/WebCore/svg/properties/SVGAnimatedPropertyAnimator.h	2019-06-06 20:23:51 UTC (rev 246169)
+++ trunk/Source/WebCore/svg/properties/SVGAnimatedPropertyAnimator.h	2019-06-06 21:56:55 UTC (rev 246170)
@@ -85,13 +85,13 @@
         if (!m_animated->isAnimating())
             return;
 
+        applyAnimatedPropertyChange(targetElement);
+        if (isAnimatedStylePropertyAniamtor(targetElement))
+            removeAnimatedStyleProperty(targetElement);
+
         m_animated->stopAnimation();
         for (auto& instance : m_animatedInstances)
             instance->instanceStopAnimation();
-
-        applyAnimatedPropertyChange(targetElement);
-        if (isAnimatedStylePropertyAniamtor(targetElement))
-            removeAnimatedStyleProperty(targetElement);
     }
 
     Optional<float> calculateDistance(SVGElement* targetElement, const String& from, const String& to) const override
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to