Bug 1518932 - Convert menulist to custom element r=paolo

This custom element replaces XBL <content> usage by directly prepend the two needed child nodes when the element is connected.

This is doable because

- There isn't any direct access of child nodes under <menulist>. Everyone seems to access via .menupopup, which is usually the only child.
- We don't need to move the children under <menulist>. If we need to and if the child is a <xbl:children> (which could happen if <menulist> is inside an XBL <content> that just get cloned to the document), the layout will get very confused and crash (see finding in bug 1514926)

Differential Revision: https://phabricator.services.mozilla.com/D16149

--HG--
rename : toolkit/content/widgets/menulist.xml => toolkit/content/widgets/menulist.js
extra : moz-landing-system : lando

browser/components/places/tests/browser/browser_bookmarkProperties_remember_folders.js

@@ -47,7 +47,7 @@ async function assertRecentFolders(expectedGuids, msg) {
   let actualGuids = [];
   function getGuids() {
     actualGuids = [];
-    const folderMenuPopup = document.getElementById("editBMPanel_folderMenuList").children[0];
+    const folderMenuPopup = document.getElementById("editBMPanel_folderMenuList").menupopup;
 
     let separatorFound = false;
     // The list of folders goes from editBMPanel_foldersSeparator to the end.

browser/components/preferences/browserLanguages.js

@@ -181,7 +181,7 @@ class OrderedListBox {
 class SortedItemSelectList {
   constructor({menulist, button, onSelect, onChange, compareFn}) {
     this.menulist = menulist;
-    this.popup = menulist.firstElementChild;
+    this.popup = menulist.menupopup;
     this.button = button;
     this.compareFn = compareFn;
     this.items = [];

browser/components/preferences/in-content/tests/browser_browser_languages_subdialog.js

@@ -141,7 +141,7 @@ function assertLocaleOrder(list, locales) {
 }
 
 function assertAvailableLocales(list, locales) {
-  let items = Array.from(list.firstElementChild.children);
+  let items = Array.from(list.menupopup.children);
   let listLocales = items
     .filter(item => item.value && item.value != "search");
   is(listLocales.length, locales.length, "The right number of locales are available");
@@ -171,7 +171,7 @@ function assertTelemetryRecorded(events) {
 }
 
 function selectLocale(localeCode, available, dialogDoc) {
-  let [locale] = Array.from(available.firstElementChild.children)
+  let [locale] = Array.from(available.menupopup.children)
     .filter(item => item.value == localeCode);
   available.selectedItem = locale;
   dialogDoc.getElementById("add").doCommand();
@@ -181,7 +181,7 @@ async function openDialog(doc, search = false) {
   let dialogLoaded = promiseLoadSubDialog(BROWSER_LANGUAGES_URL);
   if (search) {
     doc.getElementById("defaultBrowserLanguageSearch").doCommand();
-    doc.getElementById("defaultBrowserLanguage").firstElementChild.hidePopup();
+    doc.getElementById("defaultBrowserLanguage").menupopup.hidePopup();
   } else {
     doc.getElementById("manageBrowserLanguagesButton").doCommand();
   }
@@ -242,13 +242,13 @@ add_task(async function testDisabledBrowserLanguages() {
   assertAvailableLocales(available, ["fr"]);
 
   // Search for more languages.
-  available.firstElementChild.lastElementChild.doCommand();
-  available.firstElementChild.hidePopup();
+  available.menupopup.lastElementChild.doCommand();
+  available.menupopup.hidePopup();
   await waitForMutation(
-    available.firstElementChild,
+    available.menupopup,
     {childList: true},
     target =>
-      Array.from(available.firstElementChild.children)
+      Array.from(available.menupopup.children)
         .some(locale => locale.value == "pl"));
 
   // pl is now available since it is available remotely.
@@ -486,7 +486,7 @@ add_task(async function testInstallFromAMO() {
 
   if (available.itemCount == 1) {
     await waitForMutation(
-      available.firstElementChild,
+      available.menupopup,
       {childList: true},
       target => available.itemCount > 1);
   }
@@ -554,7 +554,7 @@ add_task(async function testInstallFromAMO() {
   // Wait for the available langpacks to load.
   if (available.itemCount == 1) {
     await waitForMutation(
-      available.firstElementChild,
+      available.menupopup,
       {childList: true},
       target => available.itemCount > 1);
   }
@@ -599,10 +599,10 @@ add_task(async function testDownloadEnabled() {
   let doc = gBrowser.contentDocument;
 
   let defaultMenulist = doc.getElementById("defaultBrowserLanguage");
-  ok(hasSearchOption(defaultMenulist.firstChild), "There's a search option in the General pane");
+  ok(hasSearchOption(defaultMenulist.menupopup), "There's a search option in the General pane");
 
   let { available } = await openDialog(doc, false);
-  ok(hasSearchOption(available.firstChild), "There's a search option in the dialog");
+  ok(hasSearchOption(available.menupopup), "There's a search option in the dialog");
 
   BrowserTestUtils.removeTab(gBrowser.selectedTab);
 });
@@ -620,10 +620,10 @@ add_task(async function testDownloadDisabled() {
   let doc = gBrowser.contentDocument;
 
   let defaultMenulist = doc.getElementById("defaultBrowserLanguage");
-  ok(!hasSearchOption(defaultMenulist.firstChild), "There's no search option in the General pane");
+  ok(!hasSearchOption(defaultMenulist.menupopup), "There's no search option in the General pane");
 
   let { available } = await openDialog(doc, false);
-  ok(!hasSearchOption(available.firstChild), "There's no search option in the dialog");
+  ok(!hasSearchOption(available.menupopup), "There's no search option in the dialog");
 
   BrowserTestUtils.removeTab(gBrowser.selectedTab);
 });
@@ -654,7 +654,7 @@ add_task(async function testReorderMainPane() {
   is(messageBar.hidden, true, "The message bar is hidden at first");
 
   let available = doc.getElementById("defaultBrowserLanguage");
-  let availableLocales = Array.from(available.firstElementChild.children);
+  let availableLocales = Array.from(available.menupopup.children);
   let availableCodes = availableLocales.map(item => item.value).sort().join(",");
   is(availableCodes, "en-US,fr,he,pl",
      "All of the available locales are listed");
@@ -663,7 +663,7 @@ add_task(async function testReorderMainPane() {
 
   let hebrew = availableLocales[availableLocales.findIndex(item => item.value == "he")];
   hebrew.click();
-  available.firstElementChild.hidePopup();
+  available.menupopup.hidePopup();
 
   await BrowserTestUtils.waitForCondition(
     () => !messageBar.hidden, "Wait for message bar to show");

browser/components/preferences/in-content/tests/browser_change_app_handler.js

@@ -30,7 +30,7 @@ add_task(async function() {
 
   let list = ourItem.querySelector(".actionsMenu");
 
-  let chooseItem = list.firstElementChild.querySelector(".choose-app-item");
+  let chooseItem = list.menupopup.querySelector(".choose-app-item");
   let dialogLoadedPromise = promiseLoadSubDialog("chrome://global/content/appPicker.xul");
   let cmdEvent = win.document.createEvent("xulcommandevent");
   cmdEvent.initCommandEvent("command", true, true, win, 0, false, false, false, false, null, 0);
@@ -58,7 +58,7 @@ add_task(async function() {
   // Now try to 'manage' this list:
   dialogLoadedPromise = promiseLoadSubDialog("chrome://browser/content/preferences/applicationManager.xul");
 
-  let manageItem = list.firstElementChild.querySelector(".manage-app-item");
+  let manageItem = list.menupopup.querySelector(".manage-app-item");
   cmdEvent = win.document.createEvent("xulcommandevent");
   cmdEvent.initCommandEvent("command", true, true, win, 0, false, false, false, false, null, 0);
   manageItem.dispatchEvent(cmdEvent);

browser/components/preferences/in-content/tests/browser_permissions_dialog.js

@@ -19,9 +19,7 @@ function checkPermissionItem(origin, state) {
   Assert.equal(label.value, origin);
 
   let menulist = doc.getElementsByTagName("menulist")[0];
-  let selectedIndex = menulist.selectedIndex;
-  let selectedItem = menulist.querySelectorAll("menuitem")[selectedIndex];
-  Assert.equal(selectedItem.value, state);
+  Assert.equal(menulist.value, state);
 }
 
 async function openPermissionsDialog() {

browser/components/preferences/sitePermissions.js

@@ -264,11 +264,9 @@ var gSitePermissionsManager = {
     hbox.appendChild(website);
 
     let menulist = document.createXULElement("menulist");
-    let menupopup = document.createXULElement("menupopup");
     menulist.setAttribute("flex", "1");
     menulist.setAttribute("width", "50");
     menulist.setAttribute("class", "website-status");
-    menulist.appendChild(menupopup);
     let states = SitePermissions.getAvailableStates(permission.type);
     for (let state of states) {
       // Work around the (rare) edge case when a user has changed their
@@ -280,15 +278,13 @@ var gSitePermissionsManager = {
       } else if (state == SitePermissions.UNKNOWN) {
         continue;
       }
-      let m = document.createXULElement("menuitem");
+      let m = menulist.appendItem(undefined, state);
       document.l10n.setAttributes(m, this._getCapabilityString(state));
-      m.setAttribute("value", state);
-      menupopup.appendChild(m);
     }
     menulist.value = permission.capability;
 
     menulist.addEventListener("select", () => {
-      this.onPermissionChange(permission, Number(menulist.selectedItem.value));
+      this.onPermissionChange(permission, Number(menulist.value));
     });
 
     row.appendChild(hbox);

browser/modules/SelectionChangedMenulist.jsm

@@ -12,7 +12,7 @@ class SelectionChangedMenulist {
   // is hiding. This matches the behaviour of MacOS and Linux more closely.
 
   constructor(menulist, onCommand) {
-    let popup = menulist.firstElementChild;
+    let popup = menulist.menupopup;
     let lastEvent;
 
     menulist.addEventListener("command", event => {

browser/modules/webrtcUI.jsm

@@ -606,8 +606,9 @@ function prompt(aBrowser, aRequest) {
         // Removing the child nodes of the menupopup doesn't clear the value
         // attribute of the menulist. This can have unfortunate side effects
         // when the list is rebuilt with a different content, so we remove
-        // the value attribute explicitly.
+        // the value attribute and unset the selectedItem explicitly.
         menupopup.parentNode.removeAttribute("value");
+        menupopup.parentNode.selectedItem = null;
 
         for (let device of devices)
           addDeviceToList(menupopup, device.name, device.deviceIndex);
@@ -628,6 +629,14 @@ function prompt(aBrowser, aRequest) {
         while (menupopup.lastChild) {
           menupopup.removeChild(menupopup.lastChild);
         }
+
+        // Removing the child nodes of the menupopup doesn't clear the value
+        // attribute of the menulist. This can have unfortunate side effects
+        // when the list is rebuilt with a different content, so we remove
+        // the value attribute and unset the selectedItem explicitly.
+        menupopup.parentNode.removeAttribute("value");
+        menupopup.parentNode.selectedItem = null;
+
         let label = doc.getElementById("webRTC-selectWindow-label");
         const gumStringId = "getUserMedia.selectWindowOrScreen";
         label.setAttribute("value",

toolkit/content/customElements.js

@@ -316,6 +316,7 @@ if (!isDummyDocument) {
 
   for (let [tag, script] of [
     ["findbar", "chrome://global/content/elements/findbar.js"],
+    ["menulist", "chrome://global/content/elements/menulist.js"],
     ["richlistbox", "chrome://global/content/elements/richlistbox.js"],
     ["stringbundle", "chrome://global/content/elements/stringbundle.js"],
     ["printpreview-toolbar", "chrome://global/content/printPreviewToolbar.js"],

toolkit/content/jar.mn

@@ -72,7 +72,6 @@ toolkit.jar:
 *  content/global/bindings/dialog.xml          (widgets/dialog.xml)
    content/global/bindings/general.xml         (widgets/general.xml)
    content/global/bindings/menu.xml            (widgets/menu.xml)
-   content/global/bindings/menulist.xml        (widgets/menulist.xml)
    content/global/bindings/notification.xml    (widgets/notification.xml)
    content/global/bindings/popup.xml           (widgets/popup.xml)
    content/global/bindings/radio.xml           (widgets/radio.xml)
@@ -99,6 +98,7 @@ toolkit.jar:
    content/global/elements/richlistbox.js      (widgets/richlistbox.js)
    content/global/elements/marquee.css         (widgets/marquee.css)
    content/global/elements/marquee.js          (widgets/marquee.js)
+   content/global/elements/menulist.js         (widgets/menulist.js)
    content/global/elements/stringbundle.js     (widgets/stringbundle.js)
    content/global/elements/tabbox.js           (widgets/tabbox.js)
    content/global/elements/textbox.js          (widgets/textbox.js)

toolkit/content/tests/chrome/test_menulist_keynav.xul

@@ -5,8 +5,8 @@
 <window title="Menulist Key Navigation Tests"
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
 
-  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>      
-  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>      
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
 
 <button id="button1" label="One"/>
 <menulist id="list">
@@ -104,7 +104,7 @@ function pressedAgain()
   SpecialPowers.setIntPref("ui.menu.incremental_search.timeout", 0); // prevent to timeout
   keyCheck(list, "W", 2, 1, "second letter pressed");
   SpecialPowers.clearUserPref("ui.menu.incremental_search.timeout");
-  setTimeout(differentPressed, 1200); 
+  setTimeout(differentPressed, 1200);
 }
 
 function differentPressed()
@@ -155,7 +155,7 @@ function tabAndScroll()
     list.appendItem("Item" + i, "item" + i);
   }
   list.open = true;
-  is(list.getBoundingClientRect().width, list.firstChild.getBoundingClientRect().width,
+  is(list.getBoundingClientRect().width, list.menupopup.getBoundingClientRect().width,
      "menu and popup width match");
   var minScrollbarWidth = window.matchMedia("(-moz-overlay-scrollbars)").matches ? 0 : 3;
   ok(list.getBoundingClientRect().width >= list.getItemAtIndex(0).getBoundingClientRect().width + minScrollbarWidth,

toolkit/content/tests/chrome/test_menulist_position.xul

@@ -41,7 +41,7 @@ function popupShown()
 
   let index = menulist.selectedIndex;
   if (menulist.selectedItem && navigator.platform.includes("Mac")) {
-    let menulistlabel = document.getAnonymousElementByAttribute(menulist, "class", "menulist-label");
+    let menulistlabel = menulist.querySelector(".menulist-label");
     let mitemlabel = document.getAnonymousElementByAttribute(menulist.selectedItem, "class", "menu-iconic-text");
 
     ok(isWithinHalfPixel(menulistlabel.getBoundingClientRect().left,