Bug 1549931 - Only hide direct children of treecolpicker menupopups r=surkov
authorBrian Grinstead <bgrinstead@mozilla.com>
Fri, 10 May 2019 14:33:48 +0000
changeset 532248 633722f0bc3d0444d3b5f2227b7d14680897c88b
parent 532247 a1c80c3d3855ecc1f6281cb8cd5cfaa161326896
child 532249 ad447cf869e224bc25d024bff668abcd2b949b5a
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssurkov
bugs1549931
milestone68.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 1549931 - Only hide direct children of treecolpicker menupopups r=surkov Since the menuitem DOM is not anonymous anymore, without scoping the selector we end up hiding contents of menuitems as well. Differential Revision: https://phabricator.services.mozilla.com/D30521
toolkit/content/tests/widgets/tree_shared.js
toolkit/content/widgets/tree.js
--- a/toolkit/content/tests/widgets/tree_shared.js
+++ b/toolkit/content/tests/widgets/tree_shared.js
@@ -20,17 +20,17 @@ var columns_hiertree =
 
 // XXXndeakin still to add some tests for:
 //   cycler columns, checkbox cells
 
 // this test function expects a tree to have 8 rows in it when it isn't
 // expanded. The tree should only display four rows at a time. If editable,
 // the cell at row 1 and column 0 must be editable, and the cell at row 2 and
 // column 1 must not be editable.
-function testtag_tree(treeid, treerowinfoid, seltype, columnstype, testid) {
+async function testtag_tree(treeid, treerowinfoid, seltype, columnstype, testid) {
   // Stop keystrokes that aren't handled by the tree from leaking out and
   // scrolling the main Mochitests window!
   function preventDefault(event) {
     event.preventDefault();
   }
   document.addEventListener("keypress", preventDefault);
 
   var multiple = (seltype == "multiple");
@@ -43,16 +43,17 @@ function testtag_tree(treeid, treerowinf
   else
     rowInfo = convertDOMtoTreeRowInfo(treerowinfo, 0, { value: -1 });
   var columnInfo = (columnstype == "simple") ? columns_simpletree : columns_hiertree;
 
   is(tree.selType, seltype == "multiple" ? "" : seltype, testid + " seltype");
 
   // note: the functions below should be in this order due to changes made in later tests
 
+  await testtag_tree_treecolpicker(tree, columnInfo, testid);
   testtag_tree_columns(tree, columnInfo, testid);
   testtag_tree_TreeSelection(tree, testid, multiple);
   testtag_tree_TreeSelection_UI(tree, testid, multiple);
   testtag_tree_TreeView(tree, testid, rowInfo);
 
   is(tree.editable, false, "tree should not be editable");
   // currently, the editable flag means that tree editing cannot be invoked
   // by the user. However, editing can still be started with a script.
@@ -115,16 +116,44 @@ function testtag_tree(treeid, treerowinf
 
   testtag_tree_wheel(tree);
 
   document.removeEventListener("keypress", preventDefault);
 
   SimpleTest.finish();
 }
 
+async function testtag_tree_treecolpicker(tree, expectedColumns, testid) {
+  testid += " ";
+
+  async function showAndHideTreecolpicker() {
+    let treecolpicker = tree.querySelector("treecolpicker");
+    let treecolpickerMenupopup = treecolpicker.querySelector("menupopup");
+    await new Promise(resolve => {
+      treecolpickerMenupopup.addEventListener("popupshown", resolve, { once: true });
+      treecolpicker.click();
+    });
+    let menuitems = treecolpicker.querySelectorAll("menuitem");
+    // Ignore the last "Restore Column Order" menu in the count:
+    is(menuitems.length - 1, expectedColumns.length, testid + "Same number of columns");
+    for (var c = 0; c < expectedColumns.length; c++) {
+      is(menuitems[c].textContent, expectedColumns[c].label, testid + "treecolpicker menu matches");
+      ok(!menuitems[c].querySelector("label").hidden, testid + "label not hidden");
+    }
+    await new Promise(resolve => {
+      treecolpickerMenupopup.addEventListener("popuphidden", resolve, { once: true });
+      treecolpickerMenupopup.hidePopup();
+    });
+  }
+
+  // Regression test for Bug 1549931 (menuitem content being hidden upon second open)
+  await showAndHideTreecolpicker();
+  await showAndHideTreecolpicker();
+}
+
 function testtag_tree_columns(tree, expectedColumns, testid) {
   testid += " ";
 
   var columns = tree.columns;
 
   is(columns instanceof TreeColumns, true, testid + "columns is a TreeColumns");
   is(columns.count, expectedColumns.length, testid + "TreeColumns count");
   is(columns.length, expectedColumns.length, testid + "TreeColumns length");
--- a/toolkit/content/widgets/tree.js
+++ b/toolkit/content/widgets/tree.js
@@ -236,17 +236,17 @@
             popupChild.setAttribute("checked", "true");
           if (currCol.primary)
             popupChild.setAttribute("disabled", "true");
           aPopup.insertBefore(popupChild, refChild);
         }
       }
 
       var hidden = !tree.enableColumnDrag;
-      aPopup.querySelectorAll(":not([colindex])").forEach((e) => { e.hidden = hidden; });
+      aPopup.querySelectorAll(":scope > :not([colindex])").forEach((e) => { e.hidden = hidden; });
     }
   }
 
   customElements.define("treecolpicker", MozTreecolPicker);
 
   class MozTreecol extends MozElements.BaseControl {
     static get inheritedAttributes() {
       return {