Bug 248545

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: AnimationsAssignee: 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 Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch none

Tyler Wilcock
Reported 2022-11-30 11:54:24 PST
We need to rename it to ENABLE(...) because certain Internals methods should be compiled conditionally based on this flag, and HAVE conditionals are not supported by our IDL codegen script. Also, in general we should be using this flag in more places.
Attachments
Patch (15.89 KB, patch)
2022-11-30 12:01 PST, Tyler Wilcock 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (16.60 KB, patch)
2022-11-30 13:04 PST, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (18.11 KB, patch)
2022-11-30 15:55 PST, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (18.80 KB, patch)
2022-11-30 16:11 PST, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (18.75 KB, patch)
2022-12-03 14:24 PST, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Tyler Wilcock
Comment 1 2022-11-30 12:01:57 PST
Created attachment 463810 [details] Patch
Tyler Wilcock
Comment 2 2022-11-30 13:04:46 PST
Created attachment 463814 [details] Patch
Tyler Wilcock
Comment 3 2022-11-30 15:55:01 PST
Created attachment 463816 [details] Patch
Tyler Wilcock
Comment 4 2022-11-30 16:11:37 PST
Created attachment 463817 [details] Patch
chris fleizach
Comment 5 2022-12-01 10:30:44 PST
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?
Tyler Wilcock
Comment 6 2022-12-01 10:55:57 PST
(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.
chris fleizach
Comment 7 2022-12-03 12:06:19 PST
Comment on attachment 463817 [details] Patch Thanks, lgtm
Tyler Wilcock
Comment 8 2022-12-03 14:24:54 PST
Created attachment 463878 [details] Patch
EWS
Comment 9 2022-12-04 13:33:52 PST
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].
Radar WebKit Bug Importer
Comment 10 2022-12-04 13:34:18 PST
<rdar://problem/102960581>
Note You need to log in before you can comment on or make changes to this bug.