Changeset 246170 in webkit

Timestamp:

Jun 6, 2019, 2:56:55 PM (7 years ago)

Author:

Said Abou-Hallawa

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/svg/animations/animate-externalResourcesRequired-no-load-event-expected.txt (added)
• LayoutTests/svg/animations/animate-externalResourcesRequired-no-load-event.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/svg/SVGExternalResourcesRequired.cpp (modified) (1 diff)
• Source/WebCore/svg/properties/SVGAnimatedPropertyAnimator.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-06-05  Said Abou-Hallawa  <sabouhallawa@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.Komori@sony.com>
 2019-06-06  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-06-05  Said Abou-Hallawa  <sabouhallawa@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  <sbarati@apple.com>
 2019-06-06  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/svg/SVGExternalResourcesRequired.cpp

@@ -56 +56 @@
     // 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);
 

trunk/Source/WebCore/svg/properties/SVGAnimatedPropertyAnimator.h

@@ -86 +86 @@
             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);
     }