Bug 1700137 - Add Search Engine is missing in address bar context menu after customization. r=harry
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 23 Mar 2021 17:54:31 +0000
changeset 572724 ac1d613a06d2b6ab8bccd18a69c72167060f2ab0
parent 572723 16e333f832452ffe2cfe92847032792a00402cd8
child 572725 ff26493b9197f47335b2017ae68db8c317807590
push id139291
push usermak77@bonardo.net
push dateTue, 23 Mar 2021 18:43:59 +0000
treeherderautoland@ac1d613a06d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersharry
bugs1700137
milestone89.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1700137 - Add Search Engine is missing in address bar context menu after customization. r=harry Differential Revision: https://phabricator.services.mozilla.com/D109480
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/tests/browser-proton/browser_add_search_engine.js
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -2800,16 +2800,20 @@ class UrlbarInput {
         this.startQuery({
           event,
         });
       }
     }
   }
 
   _on_contextmenu(event) {
+    if (UrlbarPrefs.get("browser.proton.urlbar.enabled")) {
+      this.addSearchEngineHelper.refreshContextMenu(event);
+    }
+
     // Context menu opened via keyboard shortcut.
     if (!event.button) {
       return;
     }
 
     this._maybeSelectAll();
   }
 
@@ -3538,38 +3542,23 @@ class CopyCutController {
   onEvent() {}
 }
 
 /**
  * Manages the Add Search Engine contextual menu entries.
  *
  * @note setEnginesFromBrowser must be invoked from the outside when the
  *       page provided engines list changes.
+ *       refreshContextMenu must be invoked when the context menu is opened.
  */
 class AddSearchEngineHelper {
   constructor(input) {
-    this.document = input.document;
-
+    this.input = input;
     this.shortcutButtons = input.view.oneOffSearchButtons;
 
-    let contextMenu = input.querySelector("moz-input-box").menupopup;
-    this.contextSeparator = this.document.createXULElement("menuseparator");
-    this.contextSeparator.setAttribute("anonid", "add-engine-separator");
-    this.contextSeparator.classList.add("menuseparator-add-engine");
-    this.contextSeparator.collapsed = true;
-    contextMenu.appendChild(this.contextSeparator);
-
-    // Since the contextual menu is not opened often, compared to the urlbar
-    // results panel, we update it just before showing it, instead of spending
-    // time on every page load.
-    contextMenu.addEventListener(
-      "popupshowing",
-      this._onContextMenu.bind(this)
-    );
-
     XPCOMUtils.defineLazyGetter(this, "_bundle", () =>
       Services.strings.createBundle("chrome://browser/locale/search.properties")
     );
   }
 
   /**
    * If there's more than this number of engines, the context menu offers
    * them in a submenu.
@@ -3634,17 +3623,17 @@ class AddSearchEngineHelper {
     }
     return ObjectUtils.deepEqual(
       engines1.map(e => e.title),
       engines2.map(e => e.title)
     );
   }
 
   _createMenuitem(engine, index) {
-    let elt = this.document.createXULElement("menuitem");
+    let elt = this.input.document.createXULElement("menuitem");
     elt.setAttribute("anonid", `add-engine-${index}`);
     elt.classList.add("menuitem-iconic");
     elt.classList.add("context-menu-add-engine");
     elt.setAttribute(
       "label",
       this._bundle.formatStringFromName("cmd_addFoundEngine", [engine.title])
     );
     elt.setAttribute("uri", engine.uri);
@@ -3653,34 +3642,48 @@ class AddSearchEngineHelper {
     } else {
       elt.removeAttribute("image", engine.icon);
     }
     elt.addEventListener("command", this._onCommand.bind(this));
     return elt;
   }
 
   _createMenu(engine) {
-    let elt = this.document.createXULElement("menu");
+    let elt = this.input.document.createXULElement("menu");
     elt.setAttribute("anonid", "add-engine-menu");
     elt.classList.add("menu-iconic");
     elt.classList.add("context-menu-add-engine");
     elt.setAttribute(
       "label",
       this._bundle.GetStringFromName("cmd_addFoundEngineMenu")
     );
     if (engine.icon) {
       elt.setAttribute("image", engine.icon);
     }
-    let popup = this.document.createXULElement("menupopup");
+    let popup = this.input.document.createXULElement("menupopup");
     elt.appendChild(popup);
     return elt;
   }
 
-  _refreshContextMenu() {
+  refreshContextMenu() {
     let engines = this.engines;
+
+    // Certain operations, like customization, destroy and recreate widgets,
+    // so we cannot rely on cached elements.
+    if (!this.input.querySelector(".menuseparator-add-engine")) {
+      this.contextSeparator = this.input.document.createXULElement(
+        "menuseparator"
+      );
+      this.contextSeparator.setAttribute("anonid", "add-engine-separator");
+      this.contextSeparator.classList.add("menuseparator-add-engine");
+      this.contextSeparator.collapsed = true;
+      let contextMenu = this.input.querySelector("moz-input-box").menupopup;
+      contextMenu.appendChild(this.contextSeparator);
+    }
+
     this.contextSeparator.collapsed = !engines.length;
     let curElt = this.contextSeparator;
     // Remove the previous items, if any.
     for (let elt = curElt.nextElementSibling; elt; ) {
       let nextElementSibling = elt.nextElementSibling;
       elt.remove();
       elt = nextElementSibling;
     }
@@ -3703,28 +3706,21 @@ class AddSearchEngineHelper {
         curElt.appendChild(elt);
       } else {
         curElt.insertAdjacentElement("afterend", elt);
       }
       curElt = elt;
     }
   }
 
-  _onContextMenu(event) {
-    // Ignore sub-menus.
-    if (event.target == event.currentTarget) {
-      this._refreshContextMenu();
-    }
-  }
-
   _onCommand(event) {
     this.addSearchEngine({
       uri: event.target.getAttribute("uri"),
       icon: event.target.getAttribute("image"),
     }).then(added => {
       if (added) {
         // Remove the offered engine from the list. The browser updated the
         // engines list at this point, so we just have to refresh the menu.)
-        this._refreshContextMenu();
+        this.refreshContextMenu();
       }
     });
   }
 }
--- a/browser/components/urlbar/tests/browser-proton/browser_add_search_engine.js
+++ b/browser/components/urlbar/tests/browser-proton/browser_add_search_engine.js
@@ -245,16 +245,58 @@ add_task(async function context_many() {
         let elt = popup.parentNode.getMenuItem(`add-engine-${i}`);
         Assert.equal(elt.parentNode, menu.menupopup);
         Assert.ok(BrowserTestUtils.is_visible(elt));
       }
     });
   });
 });
 
+add_task(async function context_after_customize() {
+  info("Checks the context menu after customization.");
+  let url = getRootDirectory(gTestPath) + "add_search_engine_one.html";
+  await BrowserTestUtils.withNewTab(url, async () => {
+    await UrlbarTestUtils.withContextMenu(window, async popup => {
+      info("The separator and the add engine item should be present.");
+      let elt = popup.parentNode.getMenuItem("add-engine-separator");
+      Assert.ok(BrowserTestUtils.is_visible(elt));
+
+      Assert.ok(!popup.parentNode.getMenuItem("add-engine-menu"));
+      Assert.ok(!popup.parentNode.getMenuItem("add-engine-1"));
+
+      elt = popup.parentNode.getMenuItem("add-engine-0");
+      Assert.ok(BrowserTestUtils.is_visible(elt));
+      Assert.ok(elt.label.includes("add_search_engine_0"));
+    });
+
+    let promise = BrowserTestUtils.waitForEvent(
+      gNavToolbox,
+      "customizationready"
+    );
+    gCustomizeMode.enter();
+    await promise;
+    promise = BrowserTestUtils.waitForEvent(gNavToolbox, "aftercustomization");
+    gCustomizeMode.exit();
+    await promise;
+
+    await UrlbarTestUtils.withContextMenu(window, async popup => {
+      info("The separator and the add engine item should be present.");
+      let elt = popup.parentNode.getMenuItem("add-engine-separator");
+      Assert.ok(BrowserTestUtils.is_visible(elt));
+
+      Assert.ok(!popup.parentNode.getMenuItem("add-engine-menu"));
+      Assert.ok(!popup.parentNode.getMenuItem("add-engine-1"));
+
+      elt = popup.parentNode.getMenuItem("add-engine-0");
+      Assert.ok(BrowserTestUtils.is_visible(elt));
+      Assert.ok(elt.label.includes("add_search_engine_0"));
+    });
+  });
+});
+
 function promiseEngine(expectedData, expectedEngineName) {
   info(`Waiting for engine ${expectedData}`);
   return TestUtils.topicObserved(
     "browser-search-engine-modified",
     (engine, data) => {
       info(`Got engine ${engine.wrappedJSObject.name} ${data}`);
       return (
         expectedData == data &&