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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 26 12:40:42 PDT 2022


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

--- Comment #28 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 461892
  --> https://bugs.webkit.org/attachment.cgi?id=461892
Patch

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

> Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml:71
> +  humanReadableName: "Control animated images"

It's a bit odd for the Experimental Feature to actually do the disabling. I'd expect the feature to be "allow image animation to be disabled", and then have some other affordance to actually control whether animation is on or off.

The title needs to reflect what the feature really does (and "control" is a bit vague). I'd also suggest choosing a name that starts with "image" or "animate" to make it easy to find in the UI.

> Source/WebCore/page/Page.cpp:4030
> +void Page::toggleAllAnimationsWhenDisabled(bool shouldAnimate)

The "when disabled" is hard to understand from a caller perspective. When what is disabled?

> Source/WebCore/platform/graphics/BitmapImage.cpp:73
> +    m_disableImageAnimation = settings.animatedImagesDisabled();

We're on Image here, so m_disableAnimation is fine. Or flip it and call this m_allowsAnimation

> Source/WebCore/platform/graphics/BitmapImage.h:254
> +    bool m_shouldAllowIndividualAnimation { false };

It's not clear what "individual" means here.

> Source/WebCore/platform/graphics/BitmapImage.h:255
> +    bool m_isPageAnimationEnabled { false };

This is unused.

> Source/WebCore/rendering/style/RenderStyle.cpp:456
> +void RenderStyle::setBackgroundImageAnimationState(bool shouldAnimate)

I'm not really a fan of poking state into Images inside styles. Can we instead add a way for the Image to ask whether it should animate via ImageObserver/CachedImageClient?

-- 
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/20220826/e8e1aa09/attachment-0001.htm>


More information about the webkit-unassigned mailing list