Bug 1498896 - Apply documentUrlPatterns to original target document when viewTypes is set. r=mixedpuppy a=RyanVM
authorRob Wu <rob@robwu.nl>
Mon, 22 Oct 2018 14:17:29 +0000
changeset 500814 498f4d6ade77ed000192706b76b88c8683098ae0
parent 500813 92b434174fcfad9f9b6ecd2550e239ca0b8dbf13
child 500815 d8cc412ec31cc606c82c9d047ef170a3bd796207
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy, RyanVM
bugs1498896
milestone64.0
Bug 1498896 - Apply documentUrlPatterns to original target document when viewTypes is set. r=mixedpuppy a=RyanVM 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