Bug 1438490 - move overflow/underflow event handling out of toolbar.xml, r?jaws draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 01 May 2018 15:44:19 +0100
changeset 790158 eb5ce23de4d6bffc96a32b6664604fd6b7193795
parent 790006 7ef8450810693ab08e79ab0d4702de6f479e678c
push id108433
push userbmo:gijskruitbosch+bugs@gmail.com
push dateTue, 01 May 2018 14:46:11 +0000
reviewersjaws
bugs1438490
milestone61.0a1
Bug 1438490 - move overflow/underflow event handling out of toolbar.xml, r?jaws MozReview-Commit-ID: CwkCOIm9wNk
browser/components/customizableui/CustomizableUI.jsm
browser/components/customizableui/content/toolbar.xml
browser/components/customizableui/test/browser_allow_dragging_removable_false.js
--- a/browser/components/customizableui/CustomizableUI.jsm
+++ b/browser/components/customizableui/CustomizableUI.jsm
@@ -4227,16 +4227,18 @@ function XULWidgetSingleWrapper(aWidgetI
 
 const LAZY_RESIZE_INTERVAL_MS = 200;
 const OVERFLOW_PANEL_HIDE_DELAY_MS = 500;
 
 function OverflowableToolbar(aToolbarNode) {
   this._toolbar = aToolbarNode;
   this._collapsed = new Map();
   this._enabled = true;
+  this._toolbar.addEventListener("overflow", this);
+  this._toolbar.addEventListener("underflow", this);
 
   this._toolbar.setAttribute("overflowable", "true");
   let doc = this._toolbar.ownerDocument;
   this._target = this._toolbar.customizationTarget;
   this._list = doc.getElementById(this._toolbar.getAttribute("overflowtarget"));
   this._list.toolbox = this._toolbar.toolbox;
   this._list.customizationTarget = this._list;
 
@@ -4278,19 +4280,19 @@ OverflowableToolbar.prototype = {
     this._panel = doc.getElementById(panelId);
     this._panel.addEventListener("popuphiding", this);
     CustomizableUIInternal.addPanelCloseListeners(this._panel);
 
     CustomizableUI.addListener(this);
     this._addedListener = true;
 
     // The 'overflow' event may have been fired before init was called.
-    if (this._toolbar.overflowedDuringConstruction) {
-      this.onOverflow(this._toolbar.overflowedDuringConstruction);
-      this._toolbar.overflowedDuringConstruction = null;
+    if (this.overflowedDuringConstruction) {
+      this.onOverflow(this.overflowedDuringConstruction);
+      this.overflowedDuringConstruction = null;
     }
 
     this.initialized = true;
   },
 
   uninit() {
     this._toolbar.removeEventListener("overflow", this._toolbar);
     this._toolbar.removeEventListener("underflow", this._toolbar);
@@ -4313,16 +4315,32 @@ OverflowableToolbar.prototype = {
     this._panel.removeEventListener("popuphiding", this);
     CustomizableUI.removeListener(this);
     this._addedListener = false;
     CustomizableUIInternal.removePanelCloseListeners(this._panel);
   },
 
   handleEvent(aEvent) {
     switch (aEvent.type) {
+      case "overflow":
+        // Ignore vertical overflow and events from from nodes inside the toolbar.
+        if (aEvent.detail > 0 && aEvent.target == this._target) {
+          if (this.initialized) {
+            this.onOverflow(aEvent);
+          } else {
+            this.overflowedDuringConstruction = aEvent;
+          }
+        }
+        break;
+      case "underflow":
+        // Ignore vertical underflow and events from from nodes inside the toolbar.
+        if (aEvent.detail > 0 && aEvent.target == this._target) {
+          this.overflowedDuringConstruction = null;
+        }
+        break;
       case "aftercustomization":
         this._enable();
         break;
       case "mousedown":
         if (aEvent.button != 0) {
           break;
         }
         if (aEvent.target == this._chevron) {
--- a/browser/components/customizableui/content/toolbar.xml
+++ b/browser/components/customizableui/content/toolbar.xml
@@ -11,22 +11,16 @@
   <binding id="toolbar">
     <implementation>
       <field name="overflowedDuringConstruction">null</field>
 
       <constructor><![CDATA[
           let scope = {};
           ChromeUtils.import("resource:///modules/CustomizableUI.jsm", scope);
           let CustomizableUI = scope.CustomizableUI;
-          // Add an early overflow event listener that will mark if the
-          // toolbar overflowed during construction.
-          if (CustomizableUI.isAreaOverflowable(this.id)) {
-            this.addEventListener("overflow", this);
-            this.addEventListener("underflow", this);
-          }
 
           // Searching for the toolbox palette in the toolbar binding because
           // toolbars are constructed first.
           let toolbox = this.toolbox;
           if (toolbox && !toolbox.palette) {
             for (let node of toolbox.children) {
               if (node.localName == "toolbarpalette") {
                 // Hold on to the palette but remove it from the document.
@@ -39,36 +33,16 @@
 
           // pass the current set of children for comparison with placements:
           let children = Array.from(this.childNodes)
                               .filter(node => node.getAttribute("skipintoolbarset") != "true" && node.id)
                               .map(node => node.id);
           CustomizableUI.registerToolbarNode(this, children);
       ]]></constructor>
 
-      <method name="handleEvent">
-        <parameter name="aEvent"/>
-        <body><![CDATA[
-          // Ignore overflow/underflow events from from nodes inside the toolbar.
-          if (aEvent.target != this.customizationTarget) {
-            return;
-          }
-
-          if (aEvent.type == "overflow" && aEvent.detail > 0) {
-            if (this.overflowable && this.overflowable.initialized) {
-              this.overflowable.onOverflow(aEvent);
-            } else {
-              this.overflowedDuringConstruction = aEvent;
-            }
-          } else if (aEvent.type == "underflow" && aEvent.detail > 0) {
-            this.overflowedDuringConstruction = null;
-          }
-        ]]></body>
-      </method>
-
       <method name="insertItem">
         <parameter name="aId"/>
         <parameter name="aBeforeElt"/>
         <parameter name="aWrapper"/>
         <body><![CDATA[
           if (aWrapper) {
             Cu.reportError("Can't insert " + aId + ": using insertItem " +
                            "no longer supports wrapper elements.");
--- a/browser/components/customizableui/test/browser_allow_dragging_removable_false.js
+++ b/browser/components/customizableui/test/browser_allow_dragging_removable_false.js
@@ -8,17 +8,17 @@ add_task(async function() {
   let forwardButton = document.getElementById("forward-button");
   is(forwardButton.getAttribute("removable"), "false", "forward-button should not be removable");
   ok(CustomizableUI.inDefaultState, "Should start in default state.");
 
   let urlbarContainer = document.getElementById("urlbar-container");
   let placementsAfterDrag = getAreaWidgetIds(CustomizableUI.AREA_NAVBAR);
   placementsAfterDrag.splice(placementsAfterDrag.indexOf("forward-button"), 1);
   placementsAfterDrag.splice(placementsAfterDrag.indexOf("urlbar-container"), 0, "forward-button");
-  simulateItemDrag(forwardButton, urlbarContainer);
+  simulateItemDrag(forwardButton, urlbarContainer, "start");
   assertAreaPlacements(CustomizableUI.AREA_NAVBAR, placementsAfterDrag);
   ok(!CustomizableUI.inDefaultState, "Should no longer be in default state.");
   let palette = document.getElementById("customization-palette");
   simulateItemDrag(forwardButton, palette);
   is(CustomizableUI.getPlacementOfWidget("forward-button").area, CustomizableUI.AREA_NAVBAR, "forward-button was not able to move to palette");
 
   await endCustomizing();
   await resetCustomization();