Individually paused / playing animations are not effected by Play All…

… Animations and Pause All Animations https://bugs.webkit.org/show_bug.cgi?id=251738 rdar://105043493 Reviewed by Andres Gonzalez. Prior to this patch, given this sequence: 1. Load a page with a GIF animation 2. Individually pause that animation 3. Perform Play All Animations The animation does not start playing again. With this patch, RenderView::updatePlayStateForAllAnimations now overrides any individual animation state, making Play All Animations and Pause All Animations behave as expected. Test case added to fast/images/page-wide-animation-toggle.html to cover this behavior. * LayoutTests/fast/images/mac/play-pause-individual-animation-context-menu-items.html: The js-test.js and resource import for this test was wrong, so I fixed it. * LayoutTests/fast/images/page-wide-animation-toggle-expected.txt: * LayoutTests/fast/images/page-wide-animation-toggle.html: * Source/WebCore/html/HTMLImageElement.cpp: (WebCore::HTMLImageElement::setAllowsAnimation): * Source/WebCore/html/HTMLImageElement.h: * Source/WebCore/page/Page.cpp: (WebCore::Page::setImageAnimationEnabled): * Source/WebCore/rendering/RenderView.cpp: (WebCore::RenderView::updatePlayStateForAllAnimations): * Source/WebCore/html/HTMLImageElement.h: * Source/WebCore/page/FrameView.cpp: * Source/WebCore/page/FrameView.h: * Source/WebCore/page/Page.cpp: * Source/WebCore/page/Page.h: * Source/WebCore/rendering/RenderView.h: Use ENABLE(ACCESSIBILITY_ANIMATION_CONTROL) in more places (we have to, because HTMLImageElement::setAllowsAnimation is only defined under this flag, and with this patch we start using that function in RenderView::updatePlayStateForAllAnimations). Canonical link: https://commits.webkit.org/259971@main
WebKit · Feb 7, 2023 · b722c6b · b722c6b
1 parent 43c15c5
commit b722c6b
Show file tree

Hide file tree

Showing 11 changed files with 68 additions and 32 deletions.
diff --git a/LayoutTests/fast/images/mac/play-pause-individual-animation-context-menu-items.html b/LayoutTests/fast/images/mac/play-pause-individual-animation-context-menu-items.html
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <html>
 <head>
-<script src="../../resources/js-test.js"></script>
+<script src="../../../resources/js-test.js"></script>
 <style>
 #gif {
   position: absolute;
@@ -12,8 +12,8 @@
 </head>
 <body>
 
-<img id="gif" src="resources/animation.gif" />
-<img id="png" src="resources/animated-red-green-blue-repeat-infinite.png" />
+<img id="gif" src="../resources/animation.gif" />
+<img id="png" src="../resources/animated-red-green-blue-repeat-infinite.png" />
 
 <script>
 description("This test ensures that the 'Play Animation' and 'Pause Animation' context menu items appear and behave correctly when activated.");

diff --git a/LayoutTests/fast/images/page-wide-animation-toggle-expected.txt b/LayoutTests/fast/images/page-wide-animation-toggle-expected.txt
@@ -3,7 +3,13 @@ This test ensures that animation play / pause state is updated when the page-wid
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
+Pausing page-wide animation.
 PASS internals.isImageAnimating(image) became false
+Resuming page-wide image animation.
+PASS internals.isImageAnimating(image) became true
+Pausing GIF individually.
+PASS internals.isImageAnimating(image) became false
+Resuming page-wide image animation (this should cause the GIF to begin animating again, despite the fact it was previously paused individually).
 PASS internals.isImageAnimating(image) became true
 PASS successfullyParsed is true
 

diff --git a/LayoutTests/fast/images/page-wide-animation-toggle.html b/LayoutTests/fast/images/page-wide-animation-toggle.html
@@ -9,21 +9,35 @@
 <div style="will-change: transform"></div>
 
 <script>
-    description("This test ensures that animation play / pause state is updated when the page-wide 'animations disabled' flag changes.\n\n");
-    jsTestIsAsync = true;
+description("This test ensures that animation play / pause state is updated when the page-wide 'animations disabled' flag changes.\n\n");
+window.jsTestIsAsync = true;
+
+var image = document.getElementById("img");
+window.onload = async function() {
+    if (!window.internals)
+        return;
     internals.settings.setImageAnimationControlEnabled(true);
-    var image = document.getElementById("img");
 
+    debug("Pausing page-wide animation.");
     internals.setImageAnimationEnabled(false);
-    window.onload = function() {
-        shouldBecomeEqual("internals.isImageAnimating(image)", "false", function() {
-            internals.setImageAnimationEnabled(true);
-            shouldBecomeEqual("internals.isImageAnimating(image)", "true", function() {
-                internals.clearMemoryCache();
-                finishJSTest();
-            });
-        });
-    };
+    await shouldBecomeEqual("internals.isImageAnimating(image)", "false");
+
+    debug("Resuming page-wide image animation.");
+    internals.setImageAnimationEnabled(true);
+    await shouldBecomeEqual("internals.isImageAnimating(image)", "true");
+
+    debug("Pausing GIF individually.");
+    internals.pauseImageAnimation(image);
+    await shouldBecomeEqual("internals.isImageAnimating(image)", "false");
+
+    debug("Resuming page-wide image animation (this should cause the GIF to begin animating again, despite the fact it was previously paused individually).");
+    internals.setImageAnimationEnabled(true);
+    await shouldBecomeEqual("internals.isImageAnimating(image)", "true");
+
+    internals.settings.setImageAnimationControlEnabled(false);
+    internals.clearMemoryCache();
+    finishJSTest();
+};
 </script>
 </body>
 </html>
diff --git a/Source/WebCore/html/HTMLImageElement.cpp b/Source/WebCore/html/HTMLImageElement.cpp
@@ -799,7 +799,7 @@ bool HTMLImageElement::allowsAnimation() const
 }
 
 #if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
-void HTMLImageElement::setAllowsAnimation(bool allowsAnimation)
+void HTMLImageElement::setAllowsAnimation(std::optional<bool> allowsAnimation)
 {
     if (!document().settings().imageAnimationControlEnabled())
         return;
@@ -810,7 +810,7 @@ void HTMLImageElement::setAllowsAnimation(bool allowsAnimation)
             renderer->repaint();
 
         if (auto* page = document().page()) {
-            if (allowsAnimation)
+            if (allowsAnimation.value_or(false))
                 page->addIndividuallyPlayingAnimationElement(*this);
             else
                 page->removeIndividuallyPlayingAnimationElement(*this);

diff --git a/Source/WebCore/html/HTMLImageElement.h b/Source/WebCore/html/HTMLImageElement.h
@@ -164,7 +164,7 @@ class HTMLImageElement : public HTMLElement, public FormAssociatedElement, publi
 
     bool allowsAnimation() const;
 #if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
-    WEBCORE_EXPORT void setAllowsAnimation(bool);
+    WEBCORE_EXPORT void setAllowsAnimation(std::optional<bool>);
 #endif
 
 protected:

diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
@@ -2901,12 +2901,6 @@ void FrameView::resumeVisibleImageAnimations(const IntRect& visibleRect)
         renderView->resumePausedImageAnimationsIfNeeded(visibleRect);
 }
 
-void FrameView::updatePlayStateForAllAnimations(const IntRect& visibleRect)
-{
-    if (auto* renderView = m_frame->contentRenderer())
-        renderView->updatePlayStateForAllAnimations(visibleRect);
-}
-
 void FrameView::updateScriptedAnimationsAndTimersThrottlingState(const IntRect& visibleRect)
 {
     if (m_frame->isMainFrame())
@@ -2942,12 +2936,20 @@ void FrameView::resumeVisibleImageAnimationsIncludingSubframes()
     });
 }
 
+#if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
+void FrameView::updatePlayStateForAllAnimations(const IntRect& visibleRect)
+{
+    if (auto* renderView = m_frame->contentRenderer())
+        renderView->updatePlayStateForAllAnimations(visibleRect);
+}
+
 void FrameView::updatePlayStateForAllAnimationsIncludingSubframes()
 {
     applyRecursivelyWithVisibleRect([] (FrameView& frameView, const IntRect& visibleRect) {
         frameView.updatePlayStateForAllAnimations(visibleRect);
     });
 }
+#endif // ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
 
 void FrameView::updateLayerPositionsAfterScrolling()
 {

diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
@@ -335,7 +335,9 @@ class FrameView final : public AbstractFrameView {
 
     void viewportContentsChanged();
     WEBCORE_EXPORT void resumeVisibleImageAnimationsIncludingSubframes();
+#if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
     void updatePlayStateForAllAnimationsIncludingSubframes();
+#endif
 
     AtomString mediaType() const;
     WEBCORE_EXPORT void setMediaType(const AtomString&);
@@ -792,7 +794,9 @@ class FrameView final : public AbstractFrameView {
 
     void applyRecursivelyWithVisibleRect(const Function<void(FrameView& frameView, const IntRect& visibleRect)>&);
     void resumeVisibleImageAnimations(const IntRect& visibleRect);
+#if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
     void updatePlayStateForAllAnimations(const IntRect& visibleRect);
+#endif
     void updateScriptedAnimationsAndTimersThrottlingState(const IntRect& visibleRect);
 
     WEBCORE_EXPORT void adjustTiledBackingCoverage();

diff --git a/Source/WebCore/page/Page.cpp b/Source/WebCore/page/Page.cpp
@@ -2004,15 +2004,14 @@ void Page::setLoadSchedulingMode(LoadSchedulingMode mode)
 #if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
 void Page::setImageAnimationEnabled(bool enabled)
 {
-    if (m_imageAnimationEnabled == enabled || !settings().imageAnimationControlEnabled())
+    if (!settings().imageAnimationControlEnabled())
         return;
+
+    // This method overrides any individually set animation play-states (so we need to do work even if `enabled` is
+    // already equal to `m_imageAnimationEnabled` because there may be individually playing or paused images).
     m_imageAnimationEnabled = enabled;
     updatePlayStateForAllAnimations();
-
-    // If the state of isAnyAnimationAllowedToPlay is not affected by the presence of individually playing
-    // animations (because there are none), then we should update it with the new animation enabled state.
-    if (!m_individuallyPlayingAnimationElements.computeSize())
-        chrome().client().isAnyAnimationAllowedToPlayDidChange(enabled);
+    chrome().client().isAnyAnimationAllowedToPlayDidChange(enabled);
 }
 #endif
 
@@ -4124,13 +4123,13 @@ void Page::forceRepaintAllFrames()
     }
 }
 
+#if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
 void Page::updatePlayStateForAllAnimations()
 {
     if (auto* view = mainFrame().view())
         view->updatePlayStateForAllAnimationsIncludingSubframes();
 }
 
-#if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
 void Page::addIndividuallyPlayingAnimationElement(HTMLImageElement& element)
 {
     ASSERT(element.allowsAnimation());

diff --git a/Source/WebCore/page/Page.h b/Source/WebCore/page/Page.h
@@ -669,6 +669,7 @@ class Page : public Supplementable<Page>, public CanMakeWeakPtr<Page> {
     bool scriptedAnimationsSuspended() const { return m_scriptedAnimationsSuspended; }
 
 #if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
+    void updatePlayStateForAllAnimations();
     WEBCORE_EXPORT void setImageAnimationEnabled(bool);
     void addIndividuallyPlayingAnimationElement(HTMLImageElement&);
     void removeIndividuallyPlayingAnimationElement(HTMLImageElement&);
@@ -718,7 +719,6 @@ class Page : public Supplementable<Page>, public CanMakeWeakPtr<Page> {
 
     WEBCORE_EXPORT VisibilityState visibilityState() const;
     WEBCORE_EXPORT void resumeAnimatingImages();
-    void updatePlayStateForAllAnimations();
 
     void didFinishLoadingImageForElement(HTMLImageElement&);
 

diff --git a/Source/WebCore/rendering/RenderView.cpp b/Source/WebCore/rendering/RenderView.cpp
@@ -32,6 +32,7 @@
 #include "HTMLFrameSetElement.h"
 #include "HTMLHtmlElement.h"
 #include "HTMLIFrameElement.h"
+#include "HTMLImageElement.h"
 #include "HitTestResult.h"
 #include "ImageQualityController.h"
 #include "LayoutInitialContainingBlock.h"
@@ -931,6 +932,7 @@ void RenderView::resumePausedImageAnimationsIfNeeded(const IntRect& visibleRect)
         m_SVGSVGElementsWithPausedImageAnimation.remove(*svgSvgElement);
 }
 
+#if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
 static SVGSVGElement* svgSvgElementFrom(RenderElement& renderElement)
 {
     if (auto* svgSvgElement = dynamicDowncast<SVGSVGElement>(renderElement.element()))
@@ -967,6 +969,12 @@ void RenderView::updatePlayStateForAllAnimations(const IntRect& visibleRect)
                     addRendererWithPausedImageAnimations(renderElement, *cachedImage);
                 }
             } else if (image && image->isAnimated()) {
+                // Override any individual animation play state that may have been set.
+                if (auto* imageElement = dynamicDowncast<HTMLImageElement>(renderElement.element()))
+                    imageElement->setAllowsAnimation(std::nullopt);
+                else
+                    image->setAllowsAnimation(std::nullopt);
+
                 // Animations of this type require a repaint to be paused or resumed.
                 if (shouldAnimate && hasPausedAnimation) {
                     needsRepaint = true;
@@ -998,6 +1006,7 @@ void RenderView::updatePlayStateForAllAnimations(const IntRect& visibleRect)
         }
     }
 }
+#endif // ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
 
 RenderView::RepaintRegionAccumulator::RepaintRegionAccumulator(RenderView* view)
 {

diff --git a/Source/WebCore/rendering/RenderView.h b/Source/WebCore/rendering/RenderView.h
@@ -183,7 +183,9 @@ class RenderView final : public RenderBlockFlow {
     void unregisterForVisibleInViewportCallback(RenderElement&);
 
     void resumePausedImageAnimationsIfNeeded(const IntRect& visibleRect);
+#if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
     void updatePlayStateForAllAnimations(const IntRect& visibleRect);
+#endif
     void addRendererWithPausedImageAnimations(RenderElement&, CachedImage&);
     void removeRendererWithPausedImageAnimations(RenderElement&);
     void removeRendererWithPausedImageAnimations(RenderElement&, CachedImage&);