Skip to content

Commit

Permalink
Context menu items for Action and Tab are not filtered by tab URL.
Browse files Browse the repository at this point in the history
https://webkit.org/b/270420
rdar://123977080

Reviewed by Brian Weinstein.

Allow documentUrlPatterns to apply to action and tab context for menus. This prevents
menu items from appearing that don’t apply to the current document in the tab.

* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionActionCocoa.mm:
(WebKit::WebExtensionAction::platformMenuItems const): Set frameURL.
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionContextCocoa.mm:
(WebKit::WebExtensionContext::platformMenuItems const): Set frameURL.
* Source/WebKit/UIProcess/Extensions/WebExtensionMenuItem.cpp:
(WebKit::WebExtensionMenuItem::matches const): Match frameURL before the early
return for action and tab contexts. Only match if frameURL is non-null.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIMenus.mm:
(TEST(WKWebExtensionAPIMenus, ActionMenus)): Added documentUrlPatterns to one item.
(TEST(WKWebExtensionAPIMenus, TabMenus)): Ditto.

Canonical link: https://commits.webkit.org/275638@main
  • Loading branch information
xeenon committed Mar 4, 2024
1 parent 24d05ed commit 7a3236b
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1037,9 +1037,14 @@ - (void)_otherPopoverWillShow:(NSNotification *)notification
if (!extensionContext())
return @[ ];

RefPtr tab = m_tab;
if (!tab && m_window)
tab = m_window->activeTab();

WebExtensionMenuItemContextParameters contextParameters;
contextParameters.types = WebExtensionMenuItemContextType::Action;
contextParameters.tabIdentifier = m_tab ? std::optional { m_tab->identifier() } : std::nullopt;
contextParameters.tabIdentifier = tab ? std::optional(tab->identifier()) : std::nullopt;
contextParameters.frameURL = tab ? tab->url() : URL { };

return WebExtensionMenuItem::matchingPlatformMenuItems(extensionContext()->mainMenuItems(), contextParameters, webExtensionActionMenuItemTopLevelLimit);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2281,6 +2281,7 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
WebExtensionMenuItemContextParameters contextParameters;
contextParameters.types = WebExtensionMenuItemContextType::Tab;
contextParameters.tabIdentifier = tab.identifier();
contextParameters.frameURL = tab.url();

if (auto *menuItem = singleMenuItemOrExtensionItemWithSubmenu(contextParameters))
return @[ menuItem ];
Expand Down
12 changes: 6 additions & 6 deletions Source/WebKit/UIProcess/Extensions/WebExtensionMenuItem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,8 @@ bool WebExtensionMenuItem::matches(const WebExtensionMenuItemContextParameters&
return false;
};

if (matchesType({ ContextType::Action, ContextType::Tab })) {
// Additional context checks are not required for Action or Tab.
return true;
}

auto matchesPattern = [&](const auto& patterns, const URL& url) {
if (patterns.isEmpty())
if (url.isNull() || patterns.isEmpty())
return true;

for (const auto& pattern : patterns) {
Expand All @@ -118,6 +113,11 @@ bool WebExtensionMenuItem::matches(const WebExtensionMenuItemContextParameters&
if (!matchesPattern(documentPatterns(), contextParameters.frameURL))
return false;

if (matchesType({ ContextType::Action, ContextType::Tab })) {
// Additional context checks are not required for Action or Tab.
return true;
}

if (matchesType(ContextType::Link)) {
ASSERT(!contextParameters.linkURL.isNull());
if (!matchesPattern(targetPatterns(), contextParameters.linkURL))
Expand Down
22 changes: 19 additions & 3 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIMenus.mm
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ static inline void performMenuItemAction(auto *menuItem)

TEST(WKWebExtensionAPIMenus, ActionMenus)
{
TestWebKitAPI::HTTPServer server({
{ "/"_s, { { { "Content-Type"_s, "text/html"_s } }, ""_s } },
}, TestWebKitAPI::HTTPServer::Protocol::Http);

auto *backgroundScript = Util::constructScript(@[
@"browser.menus.create({",
@" id: 'top-level-1',",
Expand All @@ -193,7 +197,8 @@ static inline void performMenuItemAction(auto *menuItem)
@"browser.menus.create({",
@" id: 'top-level-3',",
@" title: 'Top Level 3',",
@" contexts: [ 'action', 'page' ]",
@" contexts: [ 'action', 'page' ],",
@" documentUrlPatterns: ['*://localhost/*']",
@"})",

@"browser.menus.create({",
Expand Down Expand Up @@ -223,6 +228,9 @@ static inline void performMenuItemAction(auto *menuItem)

EXPECT_NS_EQUAL(manager.get().yieldMessage, @"Menus Created");

[manager.get().defaultTab.mainWebView loadRequest:server.requestWithLocalhost()];
[manager runForTimeInterval:1];

auto *action = [manager.get().context actionForTab:manager.get().defaultTab];
auto *menuItems = action.menuItems;

Expand Down Expand Up @@ -298,7 +306,7 @@ static inline void performMenuItemAction(auto *menuItem)
}, TestWebKitAPI::HTTPServer::Protocol::Http);

[manager.get().defaultTab.mainWebView loadRequest:server.requestWithLocalhost()];
[manager runForTimeInterval:2];
[manager runForTimeInterval:1];

auto *action = [manager.get().context actionForTab:manager.get().defaultTab];
auto *menuItems = action.menuItems;
Expand Down Expand Up @@ -505,6 +513,10 @@ static inline void performMenuItemAction(auto *menuItem)

TEST(WKWebExtensionAPIMenus, TabMenus)
{
TestWebKitAPI::HTTPServer server({
{ "/"_s, { { { "Content-Type"_s, "text/html"_s } }, ""_s } },
}, TestWebKitAPI::HTTPServer::Protocol::Http);

auto *backgroundScript = Util::constructScript(@[
@"browser.menus.create({",
@" id: 'tab-item-1',",
Expand All @@ -515,7 +527,8 @@ static inline void performMenuItemAction(auto *menuItem)
@"browser.menus.create({",
@" id: 'tab-item-2',",
@" title: 'Tab Item 2',",
@" contexts: [ 'tab' ]",
@" contexts: [ 'tab' ],",
@" documentUrlPatterns: ['*://localhost/*']",
@"})",

@"browser.menus.create({",
Expand All @@ -541,6 +554,9 @@ static inline void performMenuItemAction(auto *menuItem)

EXPECT_NS_EQUAL(manager.get().yieldMessage, @"Menus Created");

[manager.get().defaultTab.mainWebView loadRequest:server.requestWithLocalhost()];
[manager runForTimeInterval:1];

auto *menuItems = [manager.get().context menuItemsForTab:manager.get().defaultTab];
EXPECT_EQ(menuItems.count, 1lu);

Expand Down

0 comments on commit 7a3236b

Please sign in to comment.