[Webkit-unassigned] [Bug 246737] AX: Make animated image experimental feature respect preferences

[bugzilla-daemon at webkit.org]

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

--- Comment #5 from chris fleizach <cfleizach at apple.com> ---
Comment on attachment 463087
  --> https://bugs.webkit.org/attachment.cgi?id=463087
patch

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

Apologies - did not mean to get official review on this yet, so I hadn't cleaned it up to that quality yet

>> Source/WebCore/PAL/pal/spi/cocoa/AccessibilitySupportSoftLink.cpp:32
>>  SOFT_LINK_LIBRARY_FOR_SOURCE(PAL, libAccessibility)
> 
> Wouldn’t we want this in an #if ENABLE(ACCESSIBILITY_ISOLATED_TREE) || HAVE(ACCESSIBILITY_ANIMATED_IMAGES) block?

��

>> Source/WebCore/page/SettingsBase.cpp:-491
>> -}
> 
> I'm not sure if we can remove this until we remove the experimental flag entirely. This code is important because it turns animations back on if the feature flag is disabled so users aren't stuck with a page of disabled animations.

or they can just reload the page.

but I wanted opinions on this anyway...

basically the Client side is sending over the preference for whether this is on. that bit + feature flag determines if it's on

In this case here we only have access to the final state + the new experimental state... so basically you could have a different state than with the preference

then I wondered do we even need experimental flag if we're gaiting with a preference? thoughts?

>> Source/WebCore/testing/Internals.cpp:1055
>> +        page->setImageAnimationEnabled(enabled);
> 
> Why remove:
> 
> if (page->settings().imageAnimationControlEnabled())
> 
> here? Is it no longer necessary?

this is internals only. It seemed unnecessary to tie Internals to the experimental feature flag. do you have a strong opinion?

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1129
>> +    _page->process().updateAnimatedImagesState();
> 
> Where is updateAnimatedImagesState defined? I couldn't find it in this patch or in WebKit `main`.

good question...

>> Source/WebKit/WebProcess/WebPage/WebPage.cpp:785
>> +    
> 
> Let’s not add leading whitespace like this.

��

-- 
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/20221019/69f21753/attachment.htm>