Bug 250475 – Add context menu items to play and pause individual animations

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Tyler Wilcock]

Add context menu items to play and pause individual animations.

See similar bug:

https://bugs.webkit.org/show_bug.cgi?id=247377 (Add context menu items to play all or pause all animations on the page)

[Radar WebKit Bug Importer]

<rdar://problem/104137965>

[Tyler Wilcock]

Created attachment 464457 [details]
Patch

[Tyler Wilcock]

Created attachment 464460 [details]
Patch

[Tyler Wilcock]

Created attachment 464539 [details]
Patch

[Tyler Wilcock]

Created attachment 464543 [details]
Patch

[Tyler Wilcock]

Created attachment 464547 [details]
Patch

[Tyler Wilcock]

Created attachment 464552 [details]
Patch

[Tyler Wilcock]

cc Wenson



Wenson, seems like you're the most recent person to have added a context menu item, so you might be a good person to review this. Please look this patch over when you get a moment. Thanks!

[chris fleizach]

Comment on attachment 464552 [details]
Patch

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

> Source/WebCore/page/ContextMenuController.cpp:1583
> +        case ContextMenuItemTagPlayAnimation:

should these be wrapped in

#if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)

[Tyler Wilcock]

Created attachment 464775 [details]
Patch

[Tyler Wilcock]

(In reply to chris fleizach from comment #9)
> Comment on attachment 464552 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=464552&action=review
> 
> > Source/WebCore/page/ContextMenuController.cpp:1583
> > +        case ContextMenuItemTagPlayAnimation:
> 
> should these be wrapped in
> 
> #if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
We need to handle them in the switch regardless of ENABLE(ACCESSIBILITY_ANIMATION_CONTROL) because we can't gate the existence of these enum variants on this compile time flag (at least, it seems awkward and not how other context menu items are implemented). Gating the existence of the enum variant on the compile-time flag is awkward because of this static assert:

https://github.com/WebKit/WebKit/blob/df7bdf36f1cca4ea2e146d616a0cb821d98f82f5/Source/WebKit/Shared/API/c/WKContextMenuItem.cpp#L177#L262

Which requires the enum variant to exist. The pattern I've used in this patch matches that of existing context menu items (ContextMenuItemTagTranslate is a good example — it exists regardless of HAVE(TRANSLATION_UI_SERVICES) but is only allowed to be populated if said flag is enabled).

I did find a few more places we could use the flag (including the one you called out), so I've uploaded a new version of the patch.

[EWS]

Committed 259654@main (b0d7dee51245): <https://commits.webkit.org/259654@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 464775 [details].