Bug 1498896 - Apply documentUrlPatterns to original target document when viewTypes is set r=mixedpuppy
authorRob Wu <rob@robwu.nl>
Mon, 22 Oct 2018 14:17:29 +0000
changeset 490689 ffbc071b41bd30784c817593b200f9020c51c421
parent 490688 9234d32ec23c45c8ba1ebce723d57adcf784fc57
child 490690 9a6f5e359f1d3a281253b5cfa18f4f7502dd4d5e
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersmixedpuppy
bugs1498896
milestone64.0a1
Bug 1498896 - Apply documentUrlPatterns to original target document when viewTypes is set r=mixedpuppy Usually, documentUrlPatterns applies to the URL of the document where the context menu is opened. In some cases, there is no document, such as menus on browser UI (extension action buttons, tools menu, tabs). In these cases, `documentUrlPatterns` matches the (active) tab's URL. This causes ambiguity when `browser.menus.overrideContext` is used to change the context to the "tab" context. Before this patch, `documentUrlPatterns` applied to the tab's URL. After this patch, `documentUrlPatterns` applies to the URL of the document where the menu was opened, *if* `viewTypes` is also set. Using this property is a strong signal from the extension that the menu is meant to be shown in a document rather than browser UI, so extensions can reasonably expect `documentUrlPatterns` to match the original document's URL instead of the URL of the spoofed tab context. There was no existing test coverage for documentUrlPatterns on tab contexts, so this does not only add tests for documentUrlPatterns on overridden contexts (browser_ext_menus_replace_menu_context.js), but also documentUrlPatterns in normal tab contexts (browser_ext_menus.js). Differential Revision: https://phabricator.services.mozilla.com/D9249
browser/components/extensions/parent/ext-menus.js
browser/components/extensions/test/browser/browser_ext_menus.js
browser/components/extensions/test/browser/browser_ext_menus_replace_menu_context.js
--- a/browser/components/extensions/parent/ext-menus.js
+++ b/browser/components/extensions/parent/ext-menus.js
@@ -67,16 +67,17 @@ var gMenuBuilder = {
     let {webExtContextData} = contextData;
     if (!webExtContextData || !webExtContextData.overrideContext) {
       return contextData;
     }
     let contextDataBase = {
       menu: contextData.menu,
       // eslint-disable-next-line no-use-before-define
       originalViewType: getContextViewType(contextData),
+      originalViewUrl: contextData.inFrame ? contextData.frameUrl : contextData.pageUrl,
       webExtContextData,
     };
     if (webExtContextData.overrideContext === "bookmark") {
       return {
         ...contextDataBase,
         bookmarkId: webExtContextData.bookmarkId,
         onBookmark: true,
       };
@@ -603,16 +604,22 @@ function addMenuEventInfo(info, contextD
     }
     if (contextData.inFrame) {
       info.frameUrl = contextData.frameUrl;
     }
     if (contextData.isTextSelected) {
       info.selectionText = contextData.selectionText;
     }
   }
+  // If the context was overridden, then frameUrl should be the URL of the
+  // document in which the menu was opened (instead of undefined, even if that
+  // document is not in a frame).
+  if (contextData.originalViewUrl) {
+    info.frameUrl = contextData.originalViewUrl;
+  }
 }
 
 function MenuItem(extension, createProperties, isRoot = false) {
   this.extension = extension;
   this.children = [];
   this.parent = null;
   this.tabManager = extension.tabManager;
 
@@ -800,21 +807,33 @@ MenuItem.prototype = {
     if (!this.contexts.some(n => contexts.has(n))) {
       return false;
     }
 
     if (this.viewTypes && !this.viewTypes.includes(getContextViewType(contextData))) {
       return false;
     }
 
+    let docPattern = this.documentUrlMatchPattern;
+    // When viewTypes is specified, the menu item is expected to be restricted
+    // to documents. So let documentUrlPatterns always apply to the URL of the
+    // document in which the menu was opened. When maybeOverrideContextData
+    // changes the context, contextData.pageUrl does not reflect that URL any
+    // more, so use contextData.originalViewUrl instead.
+    if (docPattern && this.viewTypes && contextData.originalViewUrl) {
+      if (!docPattern.matches(Services.io.newURI(contextData.originalViewUrl))) {
+        return false;
+      }
+      docPattern = null; // Null it so that it won't be used with pageURI below.
+    }
+
     if (contextData.onBookmark) {
       return this.extension.hasPermission("bookmarks");
     }
 
-    let docPattern = this.documentUrlMatchPattern;
     let pageURI = Services.io.newURI(contextData[contextData.inFrame ? "frameUrl" : "pageUrl"]);
     if (docPattern && !docPattern.matches(pageURI)) {
       return false;
     }
 
     let targetPattern = this.targetUrlMatchPattern;
     if (targetPattern) {
       let targetUrls = [];
--- a/browser/components/extensions/test/browser/browser_ext_menus.js
+++ b/browser/components/extensions/test/browser/browser_ext_menus.js
@@ -228,17 +228,18 @@ add_task(async function test_tabContextM
     },
   });
 
   const second = ExtensionTestUtils.loadExtension({
     manifest: {
       permissions: ["menus"],
     },
     background() {
-      browser.menus.create({title: "gamma", contexts: ["tab"]}, () => {
+      browser.menus.create({title: "invisible", contexts: ["tab"], documentUrlPatterns: ["http://does/not/match"]});
+      browser.menus.create({title: "gamma", contexts: ["tab"], documentUrlPatterns: ["http://example.com/"]}, () => {
         browser.test.sendMessage("ready");
       });
     },
   });
 
   const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/");
   await first.startup();
   await second.startup();
--- a/browser/components/extensions/test/browser/browser_ext_menus_replace_menu_context.js
+++ b/browser/components/extensions/test/browser/browser_ext_menus_replace_menu_context.js
@@ -15,54 +15,88 @@ function checkIsDefaultMenuItemVisible(v
 }
 
 // Tests that the context of an extension menu can be changed to:
 // - tab
 // - bookmark
 add_task(async function overrideContext_with_context() {
   // Background script of the main test extension and the auxilary other extension.
   function background() {
+    const HTTP_URL = "http://example.com/?SomeTab";
     browser.test.onMessage.addListener(async (msg, tabId) => {
       browser.test.assertEq("testTabAccess", msg, `Expected message in ${browser.runtime.id}`);
       let tab = await browser.tabs.get(tabId);
       if (!tab.url) { // tabs or activeTab not active.
         browser.test.sendMessage("testTabAccessDone", "tab_no_url");
         return;
       }
       try {
         let [url] = await browser.tabs.executeScript(tabId, {
           code: "document.URL",
         });
-        browser.test.assertEq("http://example.com/?SomeTab", url, "Expected successful executeScript");
+        browser.test.assertEq(HTTP_URL, url, "Expected successful executeScript");
         browser.test.sendMessage("testTabAccessDone", "executeScript_ok");
         return;
       } catch (e) {
         browser.test.assertEq("Missing host permission for the tab", e.message, "Expected error message");
         browser.test.sendMessage("testTabAccessDone", "executeScript_failed");
       }
     });
     browser.menus.onShown.addListener((info, tab) => {
       browser.test.assertEq("tab", info.viewType, "Expected viewType at onShown");
+      browser.test.assertEq(undefined, info.linkUrl, "Expected linkUrl at onShown");
+      browser.test.assertEq(undefined, info.srckUrl, "Expected srcUrl at onShown");
       browser.test.sendMessage("onShown", {
-        menuIds: info.menuIds,
+        menuIds: info.menuIds.sort(),
         contexts: info.contexts,
         bookmarkId: info.bookmarkId,
+        pageUrl: info.pageUrl,
+        frameUrl: info.frameUrl,
         tabId: tab && tab.id,
       });
     });
     browser.menus.onClicked.addListener((info, tab) => {
       browser.test.assertEq("tab", info.viewType, "Expected viewType at onClicked");
+      browser.test.assertEq(undefined, info.linkUrl, "Expected linkUrl at onClicked");
+      browser.test.assertEq(undefined, info.srckUrl, "Expected srcUrl at onClicked");
       browser.test.sendMessage("onClicked", {
         menuItemId: info.menuItemId,
         bookmarkId: info.bookmarkId,
+        pageUrl: info.pageUrl,
+        frameUrl: info.frameUrl,
         tabId: tab && tab.id,
       });
     });
+
+    // Minimal properties to define menu items for a specific context.
     browser.menus.create({id: "tab_context", title: "tab_context", contexts: ["tab"]});
     browser.menus.create({id: "bookmark_context", title: "bookmark_context", contexts: ["bookmark"]});
+
+    // documentUrlPatterns in the tab context applies to the tab's URL.
+    browser.menus.create({id: "tab_context_http", title: "tab_context_http",
+                          contexts: ["tab"], documentUrlPatterns: [HTTP_URL]});
+    browser.menus.create({id: "tab_context_moz_unexpected", title: "tab_context_moz",
+                          contexts: ["tab"], documentUrlPatterns: ["moz-extension://*/tab.html"]});
+    // When viewTypes is present, the document's URL is matched instead.
+    browser.menus.create({id: "tab_context_viewType_http_unexpected", title: "tab_context_viewType_http",
+                          contexts: ["tab"], viewTypes: ["tab"], documentUrlPatterns: [HTTP_URL]});
+    browser.menus.create({id: "tab_context_viewType_moz", title: "tab_context_viewType_moz",
+                          contexts: ["tab"], viewTypes: ["tab"], documentUrlPatterns: ["moz-extension://*/tab.html"]});
+
+    // documentUrlPatterns is not restricting bookmark menu items.
+    browser.menus.create({id: "bookmark_context_http", title: "bookmark_context_http",
+                          contexts: ["bookmark"], documentUrlPatterns: [HTTP_URL]});
+    browser.menus.create({id: "bookmark_context_moz", title: "bookmark_context_moz",
+                          contexts: ["bookmark"], documentUrlPatterns: ["moz-extension://*/tab.html"]});
+    // When viewTypes is present, the document's URL is matched instead.
+    browser.menus.create({id: "bookmark_context_viewType_http_unexpected", title: "bookmark_context_viewType_http",
+                          contexts: ["bookmark"], viewTypes: ["tab"], documentUrlPatterns: [HTTP_URL]});
+    browser.menus.create({id: "bookmark_context_viewType_moz", title: "bookmark_context_viewType_moz",
+                          contexts: ["bookmark"], viewTypes: ["tab"], documentUrlPatterns: ["moz-extension://*/tab.html"]});
+
     browser.menus.create({id: "link_context", title: "link_context"}, () => {
       browser.test.sendMessage("menu_items_registered");
     });
 
     if (browser.runtime.id === "@menu-test-extension") {
       browser.tabs.create({url: "tab.html"});
     }
   }
@@ -104,16 +138,18 @@ add_task(async function overrideContext_
         document.addEventListener("contextmenu", () => {
           browser.menus.overrideContext(testCases.shift());
           browser.test.sendMessage("oncontextmenu_in_dom");
         });
 
         browser.test.sendMessage("setup_ready", {
           bookmarkId: bookmark.id,
           tabId: tab.id,
+          httpUrl: tab.url,
+          extensionUrl: document.URL,
         });
       },
     },
     background,
   });
 
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/?SomeTab");
 
@@ -125,37 +161,57 @@ add_task(async function overrideContext_
     background,
   });
   await otherExtension.startup();
   await otherExtension.awaitMessage("menu_items_registered");
 
   await extension.startup();
   await extension.awaitMessage("menu_items_registered");
 
-  let {bookmarkId, tabId} = await extension.awaitMessage("setup_ready");
+  let {bookmarkId, tabId, httpUrl, extensionUrl} = await extension.awaitMessage("setup_ready");
   info(`Set up test with tabId=${tabId} and bookmarkId=${bookmarkId}.`);
 
   {
     // Test case 1: context=tab
     let menu = await openContextMenu("a");
     await extension.awaitMessage("oncontextmenu_in_dom");
     for (let ext of [extension, otherExtension]) {
       info(`Testing menu from ${ext.id} after changing context to tab`);
       Assert.deepEqual(await ext.awaitMessage("onShown"), {
-        menuIds: ["tab_context"],
+        menuIds: [
+          "tab_context",
+          "tab_context_http",
+          "tab_context_viewType_moz",
+        ],
         contexts: ["tab"],
         bookmarkId: undefined,
+        pageUrl: undefined, // because extension has no host permissions.
+        frameUrl: extensionUrl,
         tabId,
       }, "Expected onShown details after changing context to tab");
     }
+    let topLevels = menu.getElementsByAttribute("ext-type", "top-level-menu");
+    is(topLevels.length, 1, "Expected top-level menu for otherExtension");
+
     Assert.deepEqual(getVisibleChildrenIds(menu), [
       `${makeWidgetId(extension.id)}-menuitem-_tab_context`,
+      `${makeWidgetId(extension.id)}-menuitem-_tab_context_http`,
+      `${makeWidgetId(extension.id)}-menuitem-_tab_context_viewType_moz`,
       `menuseparator`,
+      topLevels[0].id,
+    ], "Expected menu items after changing context to tab");
+
+    let submenu = await openSubmenu(topLevels[0]);
+    is(submenu, topLevels[0].firstElementChild, "Correct submenu opened");
+
+    Assert.deepEqual(getVisibleChildrenIds(submenu), [
       `${makeWidgetId(otherExtension.id)}-menuitem-_tab_context`,
-    ], "Expected menu items after changing context to tab");
+      `${makeWidgetId(otherExtension.id)}-menuitem-_tab_context_http`,
+      `${makeWidgetId(otherExtension.id)}-menuitem-_tab_context_viewType_moz`,
+    ], "Expected menu items in submenu after changing context to tab");
 
     extension.sendMessage("testTabAccess", tabId);
     is(await extension.awaitMessage("testTabAccessDone"),
        "executeScript_failed",
        "executeScript should fail due to the lack of permissions.");
 
     otherExtension.sendMessage("testTabAccess", tabId);
     is(await otherExtension.awaitMessage("testTabAccessDone"),
@@ -165,16 +221,18 @@ add_task(async function overrideContext_
     // Click on the menu item of the other extension to unlock host permissions.
     let menuItems = menu.getElementsByAttribute("label", "tab_context");
     is(menuItems.length, 2, "There are two menu items with label 'tab_context'");
     await closeExtensionContextMenu(menuItems[1]);
 
     Assert.deepEqual(await otherExtension.awaitMessage("onClicked"), {
       menuItemId: "tab_context",
       bookmarkId: undefined,
+      pageUrl: httpUrl,
+      frameUrl: extensionUrl,
       tabId,
     }, "Expected onClicked details after changing context to tab");
 
     extension.sendMessage("testTabAccess", tabId);
     is(await extension.awaitMessage("testTabAccessDone"),
        "executeScript_failed",
        "executeScript of extension that created the menu should still fail.");
 
@@ -195,16 +253,18 @@ add_task(async function overrideContext_
     await otherExtension.awaitMessage("onShown");
     let menuItems = menu.getElementsByAttribute("label", "tab_context");
     is(menuItems.length, 2, "There are two menu items with label 'tab_context'");
     await closeExtensionContextMenu(menuItems[0]);
 
     Assert.deepEqual(await extension.awaitMessage("onClicked"), {
       menuItemId: "tab_context",
       bookmarkId: undefined,
+      pageUrl: httpUrl,
+      frameUrl: extensionUrl,
       tabId,
     }, "Expected onClicked details after changing context to tab");
 
     extension.sendMessage("testTabAccess", tabId);
     is(await extension.awaitMessage("testTabAccessDone"),
        "executeScript_failed",
        "activeTab permission should not be available to the extension that created the menu.");
   }
@@ -212,27 +272,50 @@ add_task(async function overrideContext_
   {
     // Test case 3: context=bookmark
     let menu = await openContextMenu("a");
     await extension.awaitMessage("oncontextmenu_in_dom");
     for (let ext of [extension, otherExtension]) {
       info(`Testing menu from ${ext.id} after changing context to bookmark`);
       let shownInfo = await ext.awaitMessage("onShown");
       Assert.deepEqual(shownInfo, {
-        menuIds: ["bookmark_context"],
+        menuIds: [
+          "bookmark_context",
+          "bookmark_context_http",
+          "bookmark_context_moz",
+          "bookmark_context_viewType_moz",
+        ],
         contexts: ["bookmark"],
         bookmarkId,
+        pageUrl: undefined,
+        frameUrl: extensionUrl,
         tabId: undefined,
       }, "Expected onShown details after changing context to bookmark");
     }
+    let topLevels = menu.getElementsByAttribute("ext-type", "top-level-menu");
+    is(topLevels.length, 1, "Expected top-level menu for otherExtension");
+
     Assert.deepEqual(getVisibleChildrenIds(menu), [
       `${makeWidgetId(extension.id)}-menuitem-_bookmark_context`,
+      `${makeWidgetId(extension.id)}-menuitem-_bookmark_context_http`,
+      `${makeWidgetId(extension.id)}-menuitem-_bookmark_context_moz`,
+      `${makeWidgetId(extension.id)}-menuitem-_bookmark_context_viewType_moz`,
       `menuseparator`,
+      topLevels[0].id,
+    ], "Expected menu items after changing context to bookmark");
+
+    let submenu = await openSubmenu(topLevels[0]);
+    is(submenu, topLevels[0].firstElementChild, "Correct submenu opened");
+
+    Assert.deepEqual(getVisibleChildrenIds(submenu), [
       `${makeWidgetId(otherExtension.id)}-menuitem-_bookmark_context`,
-    ], "Expected menu items after changing context to bookmark");
+      `${makeWidgetId(otherExtension.id)}-menuitem-_bookmark_context_http`,
+      `${makeWidgetId(otherExtension.id)}-menuitem-_bookmark_context_moz`,
+      `${makeWidgetId(otherExtension.id)}-menuitem-_bookmark_context_viewType_moz`,
+    ], "Expected menu items in submenu after changing context to bookmark");
     await closeContextMenu(menu);
   }
 
   {
     // Test case 4: context=tab, invalid tabId.
     let menu = await openContextMenu("a");
     await extension.awaitMessage("oncontextmenu_in_dom");
     // When an invalid tabId is used, all extension menu logic is skipped and