Skip to content

Commit

Permalink
Web Extensions: menuId and parentMenuId is wrong in the menus API.
Browse files Browse the repository at this point in the history
https://webkit.org/b/265843
rdar://problem/119164396

Reviewed by Brian Weinstein.

Fix the keys we use for menuItemId and parentMenuItemId. Also make tab menu items
collapse into an extension titled submenu since these will often be next to other
extensions and should be grouped to prevent overloading menus. This can be shared
with the implementation we had for extension context menus.

* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionContextCocoa.mm:
(WebKit::WebExtensionContext::platformMenuItems const):
(WebKit::WebExtensionContext::singleMenuItemOrExtensionItemWithSubmenu const):
(WebKit::WebExtensionContext::addItemsToContextMenu):
* Source/WebKit/UIProcess/Extensions/WebExtensionContext.h:
* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIMenusCocoa.mm:
(WebKit::WebExtensionContextProxy::dispatchMenusClickedEvent):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIMenus.mm:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/271560@main
  • Loading branch information
xeenon committed Dec 5, 2023
1 parent 4d2e761 commit cec52d1
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1948,7 +1948,9 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
contextParameters.types = WebExtensionMenuItemContextType::Tab;
contextParameters.tabIdentifier = tab.identifier();

return WebExtensionMenuItem::matchingPlatformMenuItems(mainMenuItems(), contextParameters);
if (auto *menuItem = singleMenuItemOrExtensionItemWithSubmenu(contextParameters))
return @[ menuItem ];
return @[ ];
}

WebExtensionMenuItem* WebExtensionContext::menuItem(const String& identifier) const
Expand Down Expand Up @@ -1983,6 +1985,38 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
fireMenusClickedEventIfNeeded(menuItem, wasChecked, contextParameters);
}

CocoaMenuItem *WebExtensionContext::singleMenuItemOrExtensionItemWithSubmenu(const WebExtensionMenuItemContextParameters& contextParameters) const
{
#if USE(APPKIT)
auto *menuItems = WebExtensionMenuItem::matchingPlatformMenuItems(mainMenuItems(), contextParameters);
if (!menuItems.count)
return nil;

if (menuItems.count == 1) {
// Don't allow images for the top-level items, it isn't typical on macOS for menus.
dynamic_objc_cast<NSMenuItem>(menuItems.firstObject).image = nil;

return menuItems.firstObject;
}

auto *extensionItem = [[_WKWebExtensionMenuItem alloc] initWithTitle:extension().displayShortName() handler:^(id) { }];
auto *extensionSubmenu = [[NSMenu alloc] init];
extensionSubmenu.itemArray = menuItems;
extensionItem.submenu = extensionSubmenu;

return extensionItem;
#else
auto *menuItems = WebExtensionMenuItem::matchingPlatformMenuItems(mainMenuItems(), contextParameters);
if (!menuItems.count)
return nil;

if (menuItems.count == 1)
return menuItems.firstObject;

return [UIMenu menuWithTitle:extension().displayShortName() children:menuItems];
#endif
}

#if PLATFORM(MAC)
void WebExtensionContext::addItemsToContextMenu(WebPageProxy& page, const ContextMenuContextData& contextData, NSMenu *menu)
{
Expand Down Expand Up @@ -2054,24 +2088,8 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
if (contextParameters.types.isEmpty())
contextParameters.types.add(frameInfo.isMainFrame ? WebExtensionMenuItemContextType::Page : WebExtensionMenuItemContextType::Frame);

auto *menuItems = WebExtensionMenuItem::matchingPlatformMenuItems(mainMenuItems(), contextParameters);
if (!menuItems.count)
return;

if (menuItems.count == 1) {
// Don't allow images for the top-level items, it isn't typical on macOS for context menus.
dynamic_objc_cast<NSMenuItem>(menuItems.firstObject).image = nil;

[menu addItem:menuItems.firstObject];
return;
}

auto *extensionItem = [[NSMenuItem alloc] initWithTitle:extension().displayShortName() action:nullptr keyEquivalent:@""];
auto *extensionSubmenu = [[NSMenu alloc] init];
extensionSubmenu.itemArray = menuItems;
extensionItem.submenu = extensionSubmenu;

[menu addItem:extensionItem];
if (auto *menuItem = singleMenuItemOrExtensionItemWithSubmenu(contextParameters))
[menu addItem:menuItem];
}
#endif

Expand Down
2 changes: 2 additions & 0 deletions Source/WebKit/UIProcess/Extensions/WebExtensionContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,8 @@ class WebExtensionContext : public API::ObjectImpl<API::Object::Type::WebExtensi
WebExtensionMenuItem* menuItem(const String& identifier) const;
void performMenuItem(WebExtensionMenuItem&, const WebExtensionMenuItemContextParameters&, UserTriggered = UserTriggered::No);

CocoaMenuItem *singleMenuItemOrExtensionItemWithSubmenu(const WebExtensionMenuItemContextParameters&) const;

#if PLATFORM(MAC)
void addItemsToContextMenu(WebPageProxy&, const ContextMenuContextData&, NSMenu *);
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@
static NSString * const linkTextKey = @"linkText";
static NSString * const linkURLKey = @"linkUrl";
static NSString * const mediaTypeKey = @"mediaType";
static NSString * const menuIDKey = @"menuId";
static NSString * const menuItemIDKey = @"menuItemId";
static NSString * const pageURLKey = @"pageUrl";
static NSString * const parentMenuIDKey = @"parentMenuId";
static NSString * const parentMenuItemIDKey = @"parentMenuItemId";
static NSString * const selectionTextKey = @"selectionText";
static NSString * const srcURLKey = @"srcUrl";
static NSString * const wasCheckedKey = @"wasChecked";
Expand Down Expand Up @@ -463,10 +463,10 @@ static id toMenuIdentifierWebAPI(const String& identifier)

auto *info = [NSMutableDictionary dictionary];

info[menuIDKey] = toMenuIdentifierWebAPI(menuItemParameters.identifier);
info[menuItemIDKey] = toMenuIdentifierWebAPI(menuItemParameters.identifier);

if (menuItemParameters.parentIdentifier)
info[parentMenuIDKey] = toMenuIdentifierWebAPI(menuItemParameters.parentIdentifier.value());
info[parentMenuItemIDKey] = toMenuIdentifierWebAPI(menuItemParameters.parentIdentifier.value());

if (isCheckedType(menuItemParameters.type.value())) {
info[checkedKey] = @(menuItemParameters.checked.value_or(false));
Expand Down
Loading

0 comments on commit cec52d1

Please sign in to comment.