Bug 1420583 - fix new tab adjacency logic when using "restore defaults", r=jaws a=gchang
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 27 Nov 2017 15:23:42 +0000
changeset 445067 5be79b1b3deb680b309fb99717a82c9737211ce0
parent 445066 0bedbe646132065972b6c95bac1a411ecec8e10c
child 445068 22a20bb50c44580894278edc9a3df19e4eb1b804
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, gchang
bugs1420583
milestone58.0
Bug 1420583 - fix new tab adjacency logic when using "restore defaults", r=jaws a=gchang When using "restore defaults", Customize mode unwraps all elements. The previous implementation of _updateNewTabVisibility assumed that elements were wrapped in <toolbarpaletteitem>s when in customize mode, but this isn't true during a reset. The new implementation just looks for the <toolbarpaletteitem>s and their children as necessary. MozReview-Commit-ID: FjNJ1n8foGi
browser/base/content/tabbrowser.xml
browser/components/customizableui/test/browser_newtab_button_customizemode.js
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -5855,32 +5855,24 @@
               break;
             }
           }
         ]]></body>
       </method>
 
       <method name="_updateNewTabVisibility">
         <body><![CDATA[
-          let isCustomizing = this.tabContainer.parentNode.getAttribute("customizing") == "true";
-
-          // Confusingly, the <tabs> are never wrapped in <toolbarpaletteitem>s in customize mode,
-          // but the other items will be.
-          let sib = this.tabContainer.nextElementSibling;
-          if (isCustomizing) {
-            sib = sib && sib.firstElementChild;
-          }
-          while (sib && sib.hidden) {
-            if (isCustomizing) {
-              sib = sib.parentNode.nextElementSibling;
-              sib = sib && sib.firstElementChild;
-            } else {
-              sib = sib.nextElementSibling;
-            }
-          }
+          // Helper functions to help deal with customize mode wrapping some items
+          let wrap = n => n.parentNode.localName == "toolbarpaletteitem" ? n.parentNode : n;
+          let unwrap = n => n && n.localName == "toolbarpaletteitem" ? n.firstElementChild : n;
+
+          let sib = this.tabContainer;
+          do {
+            sib = unwrap(wrap(sib).nextElementSibling);
+          } while (sib && sib.hidden);
 
           const kAttr = "hasadjacentnewtabbutton";
           if (sib && sib.id == "new-tab-button") {
             this.tabContainer.setAttribute(kAttr, "true");
           } else {
             this.tabContainer.removeAttribute(kAttr);
           }
         ]]></body>
--- a/browser/components/customizableui/test/browser_newtab_button_customizemode.js
+++ b/browser/components/customizableui/test/browser_newtab_button_customizemode.js
@@ -99,8 +99,27 @@ add_task(async function addremove_before
   CustomizableUI.removeWidgetFromArea("home-button");
   ok(gBrowser.tabContainer.hasAttribute("hasadjacentnewtabbutton"),
     "tabs should have the adjacent newtab attribute again");
   assertNewTabButton("inner");
 
   CustomizableUI.reset();
   ok(CustomizableUI.inDefaultState, "Should be in default state");
 });
+
+/**
+  * Reset to defaults in customize mode to see if that doesn't break things.
+  */
+add_task(async function reset_before_newtab_customizemode() {
+  await startCustomizing();
+  simulateItemDrag(document.getElementById("home-button"), kGlobalNewTabButton);
+  ok(!gBrowser.tabContainer.hasAttribute("hasadjacentnewtabbutton"),
+    "tabs should no longer have the adjacent newtab attribute");
+  await endCustomizing();
+  assertNewTabButton("global");
+  await startCustomizing();
+  await gCustomizeMode.reset();
+  ok(gBrowser.tabContainer.hasAttribute("hasadjacentnewtabbutton"),
+    "tabs should have the adjacent newtab attribute again");
+  await endCustomizing();
+  assertNewTabButton("inner");
+  ok(CustomizableUI.inDefaultState, "Should be in default state");
+});