Bug 1370986 - switch to photonpanelmultiview for photon, disable off-nightly, r?mikedeboer draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 08 Jun 2017 16:38:36 +0100
changeset 591071 b32f5f4d198082cc91add805176869503a01aa4a
parent 589544 4dd1d17ba22660b8f5869a707f2e4e9f9dd5be5b
child 632413 947c1724f16c11f23efdb1b2f9408cb25c5bd241
push id62942
push userbmo:gijskruitbosch+bugs@gmail.com
push dateThu, 08 Jun 2017 15:47:52 +0000
reviewersmikedeboer
bugs1370986, 1370967, 1354086
milestone55.0a1
Bug 1370986 - switch to photonpanelmultiview for photon, disable off-nightly, r?mikedeboer This #ifdefs out the multiview for non-photon-theme, and checks for it being present in various bits of JS that interact with it. As a result, this will 'fix' the issues in this bug and in bug 1370967 for 55 as it moves off Nightly. By switching to the photonpanelmultiview, we get proper anchoring and slightly improved styling (bug 1354086 covers the rest of that), even where this code *is* enabled. bug 1370967 will still need fixing in the photonpanelmultiview / webextensions. MozReview-Commit-ID: 6x4HmyvxeRP
browser/components/customizableui/CustomizableUI.jsm
browser/components/customizableui/content/panelUI.inc.xul
browser/components/customizableui/test/browser_overflow_use_subviews.js
--- a/browser/components/customizableui/CustomizableUI.jsm
+++ b/browser/components/customizableui/CustomizableUI.jsm
@@ -1518,17 +1518,18 @@ var CustomizableUIInternal = {
     } else if (aWidget.type == "view") {
       let ownerWindow = aNode.ownerGlobal;
       let area = this.getPlacementOfWidget(aNode.id).area;
       let areaType = CustomizableUI.getAreaType(area);
       let anchor = aNode;
       if (areaType != CustomizableUI.TYPE_MENU_PANEL) {
         let wrapper = this.wrapWidget(aWidget.id).forWindow(ownerWindow);
 
-        if (wrapper && !wrapper.overflowed && wrapper.anchor) {
+        let hasMultiView = !!aNode.closest("photonpanelmultiview,panelmultiview");
+        if (wrapper && !hasMultiView && wrapper.anchor) {
           this.hidePanelForNode(aNode);
           anchor = wrapper.anchor;
         }
       }
       ownerWindow.PanelUI.showSubView(aWidget.viewId, anchor, area);
     }
   },
 
@@ -1721,19 +1722,23 @@ var CustomizableUIInternal = {
       target = target.parentNode;
     }
     if (closemenu == "none" || widgetType == "view") {
       return;
     }
 
     if (closemenu == "single") {
       let panel = this._getPanelForNode(target);
-      let multiview = panel.querySelector("panelmultiview");
+      let multiview = panel.querySelector("photonpanelmultiview,panelmultiview");
       if (multiview.showingSubView) {
-        multiview.showMainView();
+        if (multiview.instance.panelViews) {
+          multiview.goBack();
+        } else {
+          multiview.showMainView();
+        }
         return;
       }
     }
 
     // If we get here, we can actually hide the popup:
     this.hidePanelForNode(aEvent.target);
   },
 
@@ -4151,19 +4156,25 @@ OverflowableToolbar.prototype = {
 
   show() {
     if (this._panel.state == "open") {
       return Promise.resolve();
     }
     return new Promise(resolve => {
       let doc = this._panel.ownerDocument;
       this._panel.hidden = false;
-      let mainViewId = this._panel.querySelector("panelmultiview").getAttribute("mainViewId");
-      let mainView = doc.getElementById(mainViewId);
-      let contextMenu = doc.getElementById(mainView.getAttribute("context"));
+      let photonView = this._panel.querySelector("photonpanelmultiview");
+      let contextMenu;
+      if (photonView) {
+        let mainViewId = photonView.getAttribute("mainViewId");
+        let mainView = doc.getElementById(mainViewId);
+        contextMenu = doc.getElementById(mainView.getAttribute("context"));
+      } else {
+        contextMenu = doc.getElementById(this._panel.getAttribute("context"));
+      }
       gELS.addSystemEventListener(contextMenu, "command", this, true);
       let anchor = doc.getAnonymousElementByAttribute(this._chevron, "class", "toolbarbutton-icon");
       this._panel.openPopup(anchor || this._chevron);
       this._chevron.open = true;
 
       let overflowableToolbarInstance = this;
       this._panel.addEventListener("popupshown", function(aEvent) {
         this.addEventListener("dragover", overflowableToolbarInstance);
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -335,28 +335,35 @@
   </panelmultiview>
 </panel>
 
 <panel id="widget-overflow"
        role="group"
        type="arrow"
        noautofocus="true"
        position="bottomcenter topright"
+#ifndef MOZ_PHOTON_THEME
+       context="toolbar-context-menu"
+#endif
        hidden="true">
-  <panelmultiview mainViewId="widget-overflow-mainView">
+#ifdef MOZ_PHOTON_THEME
+  <photonpanelmultiview mainViewId="widget-overflow-mainView">
     <panelview id="widget-overflow-mainView"
                context="toolbar-context-menu">
+#endif
       <vbox id="widget-overflow-scroller">
         <vbox id="widget-overflow-list" class="widget-overflow-list"
               overflowfortoolbar="nav-bar"/>
         <toolbarseparator id="widget-overflow-fixed-separator" hidden="true"/>
         <vbox id="widget-overflow-fixed-list" class="widget-overflow-list" hidden="true"/>
       </vbox>
+#ifdef MOZ_PHOTON_THEME
     </panelview>
-  </panelmultiview>
+  </photonpanelmultiview>
+#endif
 </panel>
 
 <panel id="customization-tipPanel"
        type="arrow"
        flip="none"
        side="left"
        position="leftcenter topright"
        noautohide="true"
--- a/browser/components/customizableui/test/browser_overflow_use_subviews.js
+++ b/browser/components/customizableui/test/browser_overflow_use_subviews.js
@@ -36,18 +36,20 @@ add_task(async function check_developer_
   chevron.click();
   await shownPanelPromise;
 
   let developerView = document.getElementById("PanelUI-developer");
   let button = document.getElementById("developer-button");
   let subviewShownPromise = subviewShown(developerView);
   button.click();
   await subviewShownPromise;
-  is(developerView.closest("panel"), kOverflowPanel, "Should be inside the panel");
-  kOverflowPanel.hidePopup();
+  let hasSubviews = !!kOverflowPanel.querySelector("photonpanelmultiview");
+  let expectedPanel = hasSubviews ? kOverflowPanel : document.getElementById("customizationui-widget-panel");
+  is(developerView.closest("panel"), expectedPanel, "Should be inside the panel");
+  expectedPanel.hidePopup();
   await Promise.resolve(); // wait for popup to hide fully.
 });
 
 /**
  * This checks that non-subview-compatible items still work correctly.
  * Ideally we should make the downloads panel and bookmarks/library item
  * proper subview items, then this test can go away, and potentially we can
  * simplify some of the subview anchoring code.