Bug 1428839 - Part 4 - Add a PanelView class using a base class shared with PanelMultiView. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Thu, 18 Jan 2018 15:16:01 +0000
changeset 722193 c6fc658c774f3facc61ec4f4276a34f885cfbf8b
parent 722192 06afc9fd6f106a974b35249ede9dd459f6360b27
child 722194 ec843c63d1a7bd4fffb0e9058dd535dc6bad37d0
push id96081
push userpaolo.mozmail@amadzone.org
push dateThu, 18 Jan 2018 16:15:56 +0000
reviewersGijs
bugs1428839
milestone59.0a1
Bug 1428839 - Part 4 - Add a PanelView class using a base class shared with PanelMultiView. r=Gijs Only one method is moved to the PanelView class to simplify the keyboard navigation test. MozReview-Commit-ID: CHB5FiT9ptn
browser/components/customizableui/PanelMultiView.jsm
browser/components/customizableui/content/panelUI.xml
browser/components/customizableui/test/browser_panel_keyboard_navigation.js
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -36,17 +36,20 @@
  *
  * If the <panelmultiview> element is "ephemeral", imported subviews will be
  * moved out again to the element specified by the viewCacheId attribute, so
  * that the panel element can be removed safely.
  */
 
 "use strict";
 
-this.EXPORTED_SYMBOLS = ["PanelMultiView"];
+this.EXPORTED_SYMBOLS = [
+  "PanelMultiView",
+  "PanelView",
+];
 
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "AppConstants",
   "resource://gre/modules/AppConstants.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "BrowserUtils",
   "resource://gre/modules/BrowserUtils.jsm");
@@ -55,35 +58,76 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 
 const TRANSITION_PHASES = Object.freeze({
   START: 1,
   PREPARE: 2,
   TRANSITION: 3,
   END: 4
 });
 
+let gNodeToObjectMap = new WeakMap();
+
+/**
+ * Allows associating an object to a node lazily using a weak map.
+ *
+ * Classes deriving from this one may be easily converted to Custom Elements,
+ * although they would lose the ability of being associated lazily.
+ */
+this.AssociatedToNode = class {
+  constructor(node) {
+    /**
+     * Node associated to this object.
+     */
+    this.node = node;
+  }
+
+  /**
+   * Retrieves the instance associated with the given node, constructing a new
+   * one if necessary. When the last reference to the node is released, the
+   * object instance will be garbage collected as well.
+   */
+  static forNode(node) {
+    let associatedToNode = gNodeToObjectMap.get(node);
+    if (!associatedToNode) {
+      associatedToNode = new this(node);
+      gNodeToObjectMap.set(node, associatedToNode);
+    }
+    return associatedToNode;
+  }
+
+  get document() {
+    return this.node.ownerDocument;
+  }
+
+  get window() {
+    return this.node.ownerGlobal;
+  }
+
+  /**
+   * nsIDOMWindowUtils for the window of this node.
+   */
+  get _dwu() {
+    if (this.__dwu)
+      return this.__dwu;
+    return this.__dwu = this.window.QueryInterface(Ci.nsIInterfaceRequestor)
+                                   .getInterface(Ci.nsIDOMWindowUtils);
+  }
+};
+
 /**
  * This is the implementation of the panelUI.xml XBL binding, moved to this
  * module, to make it easier to fork the logic for the newer photon structure.
  * Goals are:
  * 1. to make it easier to programmatically extend the list of panels,
  * 2. allow for navigation between panels multiple levels deep and
  * 3. maintain the pre-photon structure with as little effort possible.
  *
  * @type {PanelMultiView}
  */
-this.PanelMultiView = class {
-  get document() {
-    return this.node.ownerDocument;
-  }
-
-  get window() {
-    return this.node.ownerGlobal;
-  }
-
+this.PanelMultiView = class extends this.AssociatedToNode {
   get _panel() {
     return this.node.parentNode;
   }
 
   get showingSubView() {
     return this._showingSubView;
   }
   get _mainViewId() {
@@ -113,22 +157,16 @@ this.PanelMultiView = class {
    * @return {Boolean} |true| when the 'ephemeral' attribute is set, which means
    *                   that this instance should be ready to be thrown away at
    *                   any time.
    */
   get _ephemeral() {
     return this.node.hasAttribute("ephemeral");
   }
 
-  get _dwu() {
-    if (this.__dwu)
-      return this.__dwu;
-    return this.__dwu = this.window.QueryInterface(Ci.nsIInterfaceRequestor)
-                                   .getInterface(Ci.nsIDOMWindowUtils);
-  }
   get _screenManager() {
     if (this.__screenManager)
       return this.__screenManager;
     return this.__screenManager = Cc["@mozilla.org/gfx/screenmanager;1"]
                                     .getService(Ci.nsIScreenManager);
   }
   /**
    * @return {panelview} the currently visible subview OR the subview that is
@@ -156,23 +194,17 @@ this.PanelMultiView = class {
     return this.__keyNavigationMap;
   }
   get _multiLineElementsMap() {
     if (!this.__multiLineElementsMap)
       this.__multiLineElementsMap = new WeakMap();
     return this.__multiLineElementsMap;
   }
 
-  constructor(xulNode, testMode = false) {
-    this.node = xulNode;
-    // If `testMode` is `true`, the consumer is only interested in accessing the
-    // methods of this instance. (E.g. in unit tests.)
-    if (testMode)
-      return;
-
+  connect() {
     this.knownViews = new Set(this.node.getElementsByTagName("panelview"));
     this.openViews = [];
     this._mainViewHeight = 0;
     this.__transitioning = this._ignoreMutations = this._showingSubView = false;
 
     const {document, window} = this;
 
     this._viewContainer =
@@ -916,17 +948,17 @@ this.PanelMultiView = class {
     let navMap = this._keyNavigationMap.get(view);
     if (!navMap) {
       navMap = {};
       this._keyNavigationMap.set(view, navMap);
     }
 
     let buttons = navMap.buttons;
     if (!buttons || !buttons.length) {
-      buttons = navMap.buttons = this._getNavigableElements(view);
+      buttons = navMap.buttons = PanelView.forNode(view).getNavigableElements();
       // Set the 'tabindex' attribute on the buttons to make sure they're focussable.
       for (let button of buttons) {
         if (!button.classList.contains("subviewbutton-back") &&
             !button.hasAttribute("tabindex")) {
           button.setAttribute("tabindex", 0);
         }
       }
     }
@@ -1010,32 +1042,16 @@ this.PanelMultiView = class {
     if (view) {
       this._keyNavigationMap.delete(view);
     } else {
       this._keyNavigationMap.clear();
     }
   }
 
   /**
-   * Retrieve the button elements from a view node that can be used for navigation
-   * using the keyboard; enabled buttons and the back button, if visible.
-   *
-   * @param  {nsIDOMNode} view
-   * @return {Array}
-   */
-  _getNavigableElements(view) {
-    let buttons = Array.from(view.querySelectorAll(".subviewbutton:not([disabled])"));
-    let dwu = this._dwu;
-    return buttons.filter(button => {
-      let bounds = dwu.getBoundsWithoutFlushing(button);
-      return bounds.width > 0 && bounds.height > 0;
-    });
-  }
-
-  /**
    * Focus the last selected element in the view, if any.
    *
    * @param {panelview} view the view in which to update keyboard focus.
    */
   _updateKeyboardFocus(view) {
     let navMap = this._keyNavigationMap.get(view);
     if (navMap && navMap.selected && navMap.selected.get()) {
       navMap.selected.get().focus();
@@ -1110,8 +1126,28 @@ this.PanelMultiView = class {
 
     // Now we can make all the necessary DOM changes at once.
     for (let { element, bounds } of items) {
       this._multiLineElementsMap.set(element, { bounds, textContent: element.textContent });
       element.style.height = bounds.height + "px";
     }
   }
 };
+
+/**
+ * This is associated to <panelview> elements.
+ */
+this.PanelView = class extends this.AssociatedToNode {
+  /**
+   * Retrieves the button elements that can be used for navigation using the
+   * keyboard, that is all enabled buttons including the back button if visible.
+   *
+   * @return {Array}
+   */
+  getNavigableElements() {
+    let buttons = Array.from(this.node.querySelectorAll(".subviewbutton:not([disabled])"));
+    let dwu = this._dwu;
+    return buttons.filter(button => {
+      let bounds = dwu.getBoundsWithoutFlushing(button);
+      return bounds.width > 0 && bounds.height > 0;
+    });
+  }
+};
--- a/browser/components/customizableui/content/panelUI.xml
+++ b/browser/components/customizableui/content/panelUI.xml
@@ -25,17 +25,18 @@
       </xul:box>
       <xul:box class="panel-viewcontainer offscreen">
         <xul:box anonid="offscreenViewStack" class="panel-viewstack"/>
       </xul:box>
     </content>
     <implementation>
       <constructor><![CDATA[
         const {PanelMultiView} = Components.utils.import("resource:///modules/PanelMultiView.jsm", {});
-        this.instance = new PanelMultiView(this);
+        this.instance = PanelMultiView.forNode(this);
+        this.instance.connect();
       ]]></constructor>
 
       <destructor><![CDATA[
         this.instance.destructor();
       ]]></destructor>
     </implementation>
   </binding>
 </bindings>
--- a/browser/components/customizableui/test/browser_panel_keyboard_navigation.js
+++ b/browser/components/customizableui/test/browser_panel_keyboard_navigation.js
@@ -1,28 +1,23 @@
 "use strict";
 
 /**
  * Test keyboard navigation in the app menu panel.
  */
 
-const {PanelMultiView} = Cu.import("resource:///modules/PanelMultiView.jsm", {});
+const {PanelView} = Cu.import("resource:///modules/PanelMultiView.jsm", {});
 const kHelpButtonId = "appMenu-help-button";
-let gHelperInstance;
-
-add_task(async function setup() {
-  gHelperInstance = new PanelMultiView(PanelUI.panel, true);
-});
 
 add_task(async function testUpDownKeys() {
   let promise = promisePanelShown(window);
   PanelUI.show();
   await promise;
 
-  let buttons = gHelperInstance._getNavigableElements(PanelUI.mainView);
+  let buttons = PanelView.forNode(PanelUI.mainView).getNavigableElements();
 
   for (let button of buttons) {
     if (button.disabled)
       continue;
     EventUtils.synthesizeKey("KEY_ArrowDown", { code: "ArrowDown" });
     Assert.equal(document.commandDispatcher.focusedElement, button,
       "The correct button should be focused after navigating downward");
   }
@@ -45,34 +40,34 @@ add_task(async function testUpDownKeys()
   await promise;
 });
 
 add_task(async function testEnterKeyBehaviors() {
   let promise = promisePanelShown(window);
   PanelUI.show();
   await promise;
 
-  let buttons = gHelperInstance._getNavigableElements(PanelUI.mainView);
+  let buttons = PanelView.forNode(PanelUI.mainView).getNavigableElements();
 
   // Navigate to the 'Help' button, which points to a subview.
   EventUtils.synthesizeKey("KEY_ArrowUp", { code: "ArrowUp" });
   let focusedElement = document.commandDispatcher.focusedElement;
   Assert.equal(focusedElement, buttons[buttons.length - 1],
     "The last button should be focused after navigating upward");
 
   promise = BrowserTestUtils.waitForEvent(PanelUI.helpView, "ViewShown");
   // Make sure the Help button is in focus.
   while (!focusedElement || !focusedElement.id || focusedElement.id != kHelpButtonId) {
     EventUtils.synthesizeKey("KEY_ArrowUp", { code: "ArrowUp" });
     focusedElement = document.commandDispatcher.focusedElement;
   }
   EventUtils.synthesizeKey("VK_RETURN", { code: "Enter" });
   await promise;
 
-  let helpButtons = gHelperInstance._getNavigableElements(PanelUI.helpView);
+  let helpButtons = PanelView.forNode(PanelUI.helpView).getNavigableElements();
   Assert.ok(helpButtons[0].classList.contains("subviewbutton-back"),
     "First button in help view should be a back button");
 
   // For posterity, check navigating the subview using up/ down arrow keys as well.
   for (let i = helpButtons.length - 1; i >= 0; --i) {
     let button = helpButtons[i];
     if (button.disabled)
       continue;
@@ -142,17 +137,17 @@ add_task(async function testLeftRightKey
   await promise;
 });
 
 add_task(async function testTabKey() {
   let promise = promisePanelShown(window);
   PanelUI.show();
   await promise;
 
-  let buttons = gHelperInstance._getNavigableElements(PanelUI.mainView);
+  let buttons = PanelView.forNode(PanelUI.mainView).getNavigableElements();
 
   for (let button of buttons) {
     if (button.disabled)
       continue;
     EventUtils.synthesizeKey("KEY_Tab", { code: "Tab" });
     Assert.equal(document.commandDispatcher.focusedElement, button,
       "The correct button should be focused after tabbing");
   }
@@ -179,17 +174,17 @@ add_task(async function testTabKey() {
   await promise;
 });
 
 add_task(async function testInterleavedTabAndArrowKeys() {
   let promise = promisePanelShown(window);
   PanelUI.show();
   await promise;
 
-  let buttons = gHelperInstance._getNavigableElements(PanelUI.mainView);
+  let buttons = PanelView.forNode(PanelUI.mainView).getNavigableElements();
   let tab = false;
 
   for (let button of buttons) {
     if (button.disabled)
       continue;
     if (tab) {
       EventUtils.synthesizeKey("KEY_Tab", { code: "Tab" });
     } else {
@@ -206,17 +201,17 @@ add_task(async function testInterleavedT
   await promise;
 });
 
 add_task(async function testSpaceDownAfterTabNavigation() {
   let promise = promisePanelShown(window);
   PanelUI.show();
   await promise;
 
-  let buttons = gHelperInstance._getNavigableElements(PanelUI.mainView);
+  let buttons = PanelView.forNode(PanelUI.mainView).getNavigableElements();
   let button;
 
   for (button of buttons) {
     if (button.disabled)
       continue;
     EventUtils.synthesizeKey("KEY_Tab", { code: "Tab" });
     if (button.id == kHelpButtonId) {
       break;