[Webkit-unassigned] [Bug 244128] Add experimental feature to disable Bitmap image animations

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 8 15:54:23 PDT 2022


https://bugs.webkit.org/show_bug.cgi?id=244128

--- Comment #53 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 462205
  --> https://bugs.webkit.org/attachment.cgi?id=462205
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=462205&action=review

> Source/WebCore/html/HTMLImageElement.cpp:783
> +void HTMLImageElement::setAllowsAnimation(bool shouldAnimate)

shouldAnimate -> allowsAnimation.

> Source/WebCore/rendering/RenderView.cpp:881
> +            return;

Should not we "continue" instead of "return"?

> Source/WebCore/rendering/RenderView.cpp:884
> +            element.repaint();

Should we check whether the background image is animated or not? Also I think we should "continue" if we repaint the element.

> LayoutTests/fast/images/disable-animations-resuming-frame.html:22
> +                debug("Image frame: " + imageFrame + " was displayed.");
> +                image.removeEventListener("webkitImageFrameReady", listener, true);
> +                resolve(frame + 1);

Can something like this work?

debug("Image frame: " + imageFrame + " was displayed.");
image.removeEventListener("webkitImageFrameReady", listener, true);
internals.pauseImageAnimation(image);
setTimeout((image, imageFrame) => {
    shouldBeTrue("internals.imageFrameIndex(image) == imageFrame");
    internals.resumeImageAnimation(image);
    resolve(frame + 1);
}, 50, image, imageFrame);

> LayoutTests/fast/images/disable-animations-resuming-frame.html:37
> +            if (i == 2) {
> +                internals.pauseImageAnimation(image);
> +                internals.resumeImageAnimation(image);
> +            }

I think this does not test anything. Pausing and resuming the animation happens in the same frame. We should allow the page to render after pausing the animation before checking whether the animation is paused or not. See my suggestion above.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220908/145c415b/attachment-0001.htm>


More information about the webkit-unassigned mailing list