Backed out changeset c0c671865b2e (bug 1699828) for browser_ext_menus…

…_replace_menu_context.js mochitest failures CLOSED TREE
dothq · Mar 23, 2021 · 393125c · 393125c
1 parent 16cc3d1
commit 393125c
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 104 deletions.
diff --git a/browser/components/extensions/parent/ext-menus.js b/browser/components/extensions/parent/ext-menus.js
@@ -168,18 +168,10 @@ var gMenuBuilder = {
           Infinity,
           false
         );
-        if (!nextSibling) {
-          // The extension menu should be rendered at the top. If we use
-          // a navigation group (on non-macOS), the extension menu should
-          // come after that to avoid styling issues.
-          if (AppConstants.platform == "macosx") {
-            nextSibling = this.xulMenu.firstElementChild;
-          } else {
-            nextSibling = this.xulMenu.querySelector(
-              ":scope > #context-sep-navigation + *"
-            );
-          }
-        }
+        // The extension menu should be rendered at the top, but after the navigation buttons.
+        nextSibling =
+          nextSibling ||
+          this.xulMenu.querySelector(":scope > #context-sep-navigation + *");
         if (
           rootElements.length &&
           showDefaults &&

diff --git a/browser/components/extensions/test/browser/browser_ext_menus_replace_menu.js b/browser/components/extensions/test/browser/browser_ext_menus_replace_menu.js
@@ -9,9 +9,9 @@ function getVisibleChildrenIds(menuElem) {
     .map(elem => elem.id || elem.tagName);
 }
 
-function checkIsLinkMenuItemVisible(visibleMenuItemIds) {
-  // In most of this test file, we open a menu on a link. Assume that all
-  // relevant menu items are shown if one link-specific menu item is shown.
+function checkIsDefaultMenuItemVisible(visibleMenuItemIds) {
+  // In this whole test file, we open a menu on a link. Assume that all
+  // default menu items are shown if one link-specific menu item is shown.
   ok(
     visibleMenuItemIds.includes("context-openlink"),
     `The default 'Open Link in New Tab' menu item should be in ${visibleMenuItemIds}.`
@@ -56,14 +56,6 @@ add_task(async function overrideContext_in_extension_tab() {
       { once: true }
     );
 
-    document.querySelector("p").addEventListener(
-      "contextmenu",
-      () => {
-        browser.menus.overrideContext({ showDefaults: true });
-      },
-      { once: true }
-    );
-
     browser.menus.create({
       id: "tab_1",
       title: "tab_1",
@@ -104,7 +96,6 @@ add_task(async function overrideContext_in_extension_tab() {
       "tab.html": `
         <!DOCTYPE html><meta charset="utf-8">
         <a href="http://example.com/">Link</a>
-        <p>Some text</p>
         <div id="shadowHost"></div>
         <script src="tab.js"></script>
       `,
@@ -133,32 +124,16 @@ add_task(async function overrideContext_in_extension_tab() {
 
       browser.menus.onShown.addListener(info => {
         browser.test.assertEq("tab", info.viewType, "Expected viewType");
-        let sortedContexts = info.contexts.sort().join(",");
-        if (info.contexts.includes("link")) {
-          browser.test.assertEq(
-            "bg_1,bg_2,tab_1,tab_2",
-            info.menuIds.join(","),
-            "Expected menu items."
-          );
-          browser.test.assertEq(
-            "all,link",
-            sortedContexts,
-            "Expected menu contexts"
-          );
-        } else if (info.contexts.includes("page")) {
-          browser.test.assertEq(
-            "bg_1,tab_1,tab_2",
-            info.menuIds.join(","),
-            "Expected menu items."
-          );
-          browser.test.assertEq(
-            "all,page",
-            sortedContexts,
-            "Expected menu contexts"
-          );
-        } else {
-          browser.test.fail(`Unexpected menu context: ${sortedContexts}`);
-        }
+        browser.test.assertEq(
+          "bg_1,bg_2,tab_1,tab_2",
+          info.menuIds.join(","),
+          "Expected menu items."
+        );
+        browser.test.assertEq(
+          "all,link",
+          info.contexts.sort().join(","),
+          "Expected menu contexts"
+        );
         browser.test.sendMessage("onShown");
       });
 
@@ -198,12 +173,6 @@ add_task(async function overrideContext_in_extension_tab() {
     `${makeWidgetId(extension.id)}-menuitem-_tab_1`,
     `${makeWidgetId(extension.id)}-menuitem-_tab_2`,
   ];
-
-  const EXPECTED_EXTENSION_MENU_IDS_NOLINK = [
-    `${makeWidgetId(extension.id)}-menuitem-_bg_1`,
-    `${makeWidgetId(extension.id)}-menuitem-_tab_1`,
-    `${makeWidgetId(extension.id)}-menuitem-_tab_2`,
-  ];
   const OTHER_EXTENSION_MENU_ID = `${makeWidgetId(
     otherExtension.id
   )}-menuitem-_other_extension_item`;
@@ -242,7 +211,7 @@ add_task(async function overrideContext_in_extension_tab() {
       "Expected extension menu items at the start."
     );
 
-    checkIsLinkMenuItemVisible(visibleMenuItemIds);
+    checkIsDefaultMenuItemVisible(visibleMenuItemIds);
 
     is(
       visibleMenuItemIds[visibleMenuItemIds.length - 1],
@@ -264,7 +233,7 @@ add_task(async function overrideContext_in_extension_tab() {
     let menu = await openContextMenu("a");
     await extension.awaitMessage("onShown");
 
-    checkIsLinkMenuItemVisible(getVisibleChildrenIds(menu));
+    checkIsDefaultMenuItemVisible(getVisibleChildrenIds(menu));
 
     let menuItems = menu.getElementsByAttribute("ext-type", "top-level-menu");
     is(menuItems.length, 1, "Expected top-level menu element for extension.");
@@ -304,51 +273,6 @@ add_task(async function overrideContext_in_extension_tab() {
     await closeContextMenu();
   }
 
-  {
-    // Tests overrideContext({showDefaults:true}) on a non-link
-    info(
-      "Expecting overrideContext to insert items after the navigation group."
-    );
-    let menu = await openContextMenu("p");
-    await extension.awaitMessage("onShown");
-
-    let visibleMenuItemIds = getVisibleChildrenIds(menu);
-    if (AppConstants.platform == "macosx") {
-      // On mac, the items should be at the top:
-      Assert.deepEqual(
-        visibleMenuItemIds.slice(0, EXPECTED_EXTENSION_MENU_IDS_NOLINK.length),
-        EXPECTED_EXTENSION_MENU_IDS_NOLINK,
-        "Expected extension menu items at the start."
-      );
-    } else {
-      // Elsewhere, they should be immediately after the navigation group:
-      Assert.deepEqual(
-        visibleMenuItemIds.slice(
-          0,
-          2 + EXPECTED_EXTENSION_MENU_IDS_NOLINK.length
-        ),
-        [
-          "context-navigation",
-          "context-sep-navigation",
-          ...EXPECTED_EXTENSION_MENU_IDS_NOLINK,
-        ],
-        "Expected extension menu items immmediately after navigation items."
-      );
-    }
-    ok(
-      visibleMenuItemIds.includes("context-savepage"),
-      "Default menu items should be there."
-    );
-
-    is(
-      visibleMenuItemIds[visibleMenuItemIds.length - 1],
-      OTHER_EXTENSION_MENU_ID,
-      "Other extension menu item should be at the end."
-    );
-
-    await closeContextMenu();
-  }
-
   // Unloading the extension will automatically close the extension's tab.html
   await extension.unload();
   await otherExtension.unload();
@@ -512,7 +436,7 @@ add_task(async function overrideContext_sidebar_edge_cases() {
     await extension.awaitMessage("oncontextmenu_in_dom");
     await extension.awaitMessage("onShown_3");
     let visibleMenuItemIds = getVisibleChildrenIds(menu);
-    checkIsLinkMenuItemVisible(visibleMenuItemIds);
+    checkIsDefaultMenuItemVisible(visibleMenuItemIds);
     ok(
       visibleMenuItemIds.includes(EXPECTED_EXTENSION_MENU_ID),
       "Expected extension menu item"