Bug 250475 - Add context menu items to play and pause individual animations
Summary: Add context menu items to play and pause individual animations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tyler Wilcock
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-01-11 13:40 PST by Tyler Wilcock
Modified: 2023-01-31 17:47 PST (History)
10 users (show)

See Also:


Attachments
Patch (38.40 KB, patch)
2023-01-11 14:25 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (39.06 KB, patch)
2023-01-11 16:47 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (39.11 KB, patch)
2023-01-18 10:05 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (39.12 KB, patch)
2023-01-18 12:23 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (40.02 KB, patch)
2023-01-18 16:34 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (39.25 KB, patch)
2023-01-18 21:49 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (40.11 KB, patch)
2023-01-30 21:47 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tyler Wilcock 2023-01-11 13:40:22 PST
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)
Comment 1 Radar WebKit Bug Importer 2023-01-11 13:40:34 PST
<rdar://problem/104137965>
Comment 2 Tyler Wilcock 2023-01-11 14:25:35 PST
Created attachment 464457 [details]
Patch
Comment 3 Tyler Wilcock 2023-01-11 16:47:40 PST
Created attachment 464460 [details]
Patch
Comment 4 Tyler Wilcock 2023-01-18 10:05:02 PST
Created attachment 464539 [details]
Patch
Comment 5 Tyler Wilcock 2023-01-18 12:23:30 PST
Created attachment 464543 [details]
Patch
Comment 6 Tyler Wilcock 2023-01-18 16:34:10 PST
Created attachment 464547 [details]
Patch
Comment 7 Tyler Wilcock 2023-01-18 21:49:35 PST
Created attachment 464552 [details]
Patch
Comment 8 Tyler Wilcock 2023-01-24 16:26:04 PST
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!
Comment 9 chris fleizach 2023-01-30 10:32:42 PST
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)
Comment 10 Tyler Wilcock 2023-01-30 21:47:23 PST
Created attachment 464775 [details]
Patch
Comment 11 Tyler Wilcock 2023-01-30 21:48:58 PST
(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.
Comment 12 EWS 2023-01-31 17:47:18 PST
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].