| Summary: | Rename HAVE(ACCESSIBILITY_ANIMATED_IMAGE_CONTROL) to ENABLE(ACCESSIBILITY_ANIMATION_CONTROL) and use it in more places | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tyler Wilcock <tyler_w> | ||||||||||||
| Component: | Animations | Assignee: | Tyler Wilcock <tyler_w> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | benjamin, cdumez, cfleizach, changseok, cmarcelo, dino, esprehn+autocc, ews-watchlist, graouts, gyuyoung.kim, webkit-bug-importer | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | Other | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Tyler Wilcock
2022-11-30 11:54:24 PST
Created attachment 463810 [details]
Patch
Created attachment 463814 [details]
Patch
Created attachment 463816 [details]
Patch
Created attachment 463817 [details]
Patch
Comment on attachment 463817 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=463817&action=review > Source/WebCore/page/Page.cpp:1996 > + if (m_imageAnimationEnabled == enabled || !settings().imageAnimationControlEnabled()) could this mean we end up in a state where user toggles off the setting, but the state is OFF, so now they can't turn animations back on? (In reply to chris fleizach from comment #5) > Comment on attachment 463817 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=463817&action=review > > > Source/WebCore/page/Page.cpp:1996 > > + if (m_imageAnimationEnabled == enabled || !settings().imageAnimationControlEnabled()) > > could this mean we end up in a state where user toggles off the setting, but > the state is OFF, so now they can't turn animations back on? Yes, but all it would take to fix this is: 1. Closing the page and re-opening it 2. Turning ImageAnimationControlEnabled back on I think this is fine since this is an experimental setting, and they are explicitly turning off the ability to control animations while they're in a state of being paused. Making this change allowed for simplification of all the callsites of this function, since without it whether the caller is allowed to `setImageAnimationEnabled` is context-dependent. Comment on attachment 463817 [details]
Patch
Thanks, lgtm
Created attachment 463878 [details]
Patch
Committed 257362@main (5bfe4c6223b7): <https://commits.webkit.org/257362@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 463878 [details]. |