Bug 1434883 - Part 3 - Prepare the main view just before opening the panel. r=Gijs
This allows the ViewShowing event for the main view to prevent the panel from opening, removing reentrancy when clicking extension buttons that don't have a panel. It also avoids setting up the main view if the panel is not opened.
MozReview-Commit-ID: Ks9fQMxjhGu
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -223,23 +223,16 @@ this.PanelMultiView = class extends this
return this.node && (this._viewShowing || this._currentSubView);
}
get _currentSubView() {
// Peek the top of the stack, but fall back to the main view if the list of
// opened views is currently empty.
let panelView = this.openViews[this.openViews.length - 1];
return (panelView && panelView.node) || this._mainView;
}
- /**
- * @return {Promise} showSubView() returns a promise, which is kept here for
- * random access.
- */
- get currentShowPromise() {
- return this._currentShowPromise || Promise.resolve();
- }
constructor(node) {
super(node);
this._openPopupPromise = Promise.resolve(false);
this._openPopupCancelCallback = () => {};
}
connect() {
@@ -268,27 +261,26 @@ this.PanelMultiView = class extends this
this._panel.addEventListener("popupshowing", this);
this._panel.addEventListener("popuppositioned", this);
this._panel.addEventListener("popuphidden", this);
this._panel.addEventListener("popupshown", this);
let cs = window.getComputedStyle(document.documentElement);
// Set CSS-determined attributes now to prevent a layout flush when we do
// it when transitioning between panels.
this._dir = cs.direction;
- this.showMainView();
// Proxy these public properties and methods, as used elsewhere by various
// parts of the browser, to this instance.
["goBack", "showMainView", "showSubView"].forEach(method => {
Object.defineProperty(this.node, method, {
enumerable: true,
value: (...args) => this[method](...args)
});
});
- ["current", "currentShowPromise", "showingSubView"].forEach(property => {
+ ["current", "showingSubView"].forEach(property => {
Object.defineProperty(this.node, property, {
enumerable: true,
get: () => this[property]
});
});
}
destructor() {
@@ -402,17 +394,20 @@ this.PanelMultiView = class extends this
// The XBL binding must be connected at this point. If this is not the
// case, the calling code should be updated to unhide the panel.
if (!this.connected) {
throw new Error("The binding for the panelmultiview element isn't" +
" connected. The containing panel may still have" +
" its display turned off by the hidden attribute.");
}
}
- // (The rest of the asynchronous preparation goes here.)
+ // Allow any of the ViewShowing handlers to prevent showing the main view.
+ if (!(await this.showMainView())) {
+ cancelCallback();
+ }
} catch (ex) {
cancelCallback();
throw ex;
}
// If a cancellation request was received there is nothing more to do.
if (!canCancel || !this.node) {
return false;
}
@@ -487,17 +482,17 @@ this.PanelMultiView = class extends this
let previous = this.openViews.pop().node;
let current = this._currentSubView;
this.showSubView(current, null, previous);
}
showMainView() {
if (!this.node || !this._mainViewId)
- return Promise.resolve();
+ return Promise.resolve(false);
return this.showSubView(this._mainView);
}
/**
* Ensures that all the panelviews, that are currently part of this instance,
* are hidden, except one specifically.
*
@@ -519,18 +514,18 @@ this.PanelMultiView = class extends this
if (!this.openViews.includes(nextPanelView))
this.openViews.push(nextPanelView);
nextPanelView.current = true;
this.showingSubView = nextPanelView.node.id != this._mainViewId;
}
- showSubView(aViewId, aAnchor, aPreviousView) {
- this._currentShowPromise = (async () => {
+ async showSubView(aViewId, aAnchor, aPreviousView) {
+ try {
// Support passing in the node directly.
let viewNode = typeof aViewId == "string" ? this.node.querySelector("#" + aViewId) : aViewId;
if (!viewNode) {
viewNode = this.document.getElementById(aViewId);
if (viewNode) {
this._viewStack.appendChild(viewNode);
} else {
throw new Error(`Subview ${aViewId} doesn't exist!`);
@@ -591,31 +586,35 @@ this.PanelMultiView = class extends this
} catch (e) {
Cu.reportError(e);
cancel = true;
}
}
if (cancel) {
this._viewShowing = null;
- return;
+ return false;
}
}
// Now we have to transition the panel. If we've got an older transition
// still running, make sure to clean it up.
await this._cleanupTransitionPhase();
if (!showingSameView && this._panel.state == "open") {
await this._transitionViews(previousViewNode, viewNode, reverse, aAnchor);
nextPanelView.focusSelectedElement();
} else {
this.hideAllViewsExcept(nextPanelView);
}
- })().catch(e => Cu.reportError(e));
- return this._currentShowPromise;
+
+ return true;
+ } catch (ex) {
+ Cu.reportError(ex);
+ return false;
+ }
}
/**
* Apply a transition to 'slide' from the currently active view to the next
* one.
* Sliding the next subview in means that the previous panelview stays where it
* is and the active panelview slides in from the left in LTR mode, right in
* RTL mode.
@@ -928,20 +927,24 @@ this.PanelMultiView = class extends this
case "popupshown":
// Now that the main view is visible, we can check the height of the
// description elements it contains.
PanelView.forNode(this._mainView).descriptionHeightWorkaround();
break;
case "popuphidden": {
// WebExtensions consumers can hide the popup from viewshowing, or
// mid-transition, which disrupts our state:
+ if (this._viewShowing) {
+ PanelView.forNode(this._viewShowing).dispatchCustomEvent("ViewHiding");
+ }
this._viewShowing = null;
this._transitioning = false;
this.node.removeAttribute("panelopen");
- this.showMainView();
+ // Raise the ViewHiding event for the current view.
+ this.hideAllViewsExcept(null);
this.window.removeEventListener("keydown", this);
this._panel.removeEventListener("mousemove", this);
this.openViews.forEach(panelView => panelView.clearNavigation());
this.openViews = [];
// Clear the main view size caches. The dimensions could be different
// when the popup is opened again, e.g. through touch mode sizing.
this._viewContainer.style.removeProperty("min-height");
--- a/browser/components/customizableui/content/panelUI.js
+++ b/browser/components/customizableui/content/panelUI.js
@@ -417,69 +417,65 @@ const PanelUI = {
// If the panelview is already selected in another PanelMultiView instance
// as a subview, make sure to properly hide it there.
let oldMultiView = viewNode.panelMultiView;
if (oldMultiView && oldMultiView.current == viewNode) {
await oldMultiView.showMainView();
}
- let viewShown = false;
- let listener = () => viewShown = true;
- viewNode.addEventListener("ViewShown", listener, {once: true});
-
let multiView = document.createElement("panelmultiview");
multiView.setAttribute("id", "customizationui-widget-multiview");
multiView.setAttribute("viewCacheId", "appMenu-viewCache");
multiView.setAttribute("mainViewId", viewNode.id);
multiView.setAttribute("ephemeral", true);
document.getElementById("appMenu-viewCache").appendChild(viewNode);
tempPanel.appendChild(multiView);
viewNode.classList.add("cui-widget-panelview");
+ let viewShown = false;
let panelRemover = () => {
viewNode.classList.remove("cui-widget-panelview");
if (viewShown) {
CustomizableUI.removePanelCloseListeners(tempPanel);
tempPanel.removeEventListener("popuphidden", panelRemover);
}
aAnchor.open = false;
// Ensure we run the destructor:
multiView.instance.destructor();
tempPanel.remove();
};
- // Wait until all the tasks needed to show a view are done.
- await multiView.currentShowPromise;
-
- if (!viewShown) {
- viewNode.removeEventListener("ViewShown", listener);
- panelRemover();
- return;
- }
-
- CustomizableUI.addPanelCloseListeners(tempPanel);
- tempPanel.addEventListener("popuphidden", panelRemover);
-
if (aAnchor.parentNode.id == "PersonalToolbar") {
tempPanel.classList.add("bookmarks-toolbar");
}
let anchor = this._getPanelAnchor(aAnchor);
if (aAnchor != anchor && aAnchor.id) {
anchor.setAttribute("consumeanchor", aAnchor.id);
}
- PanelMultiView.openPopup(tempPanel, anchor, {
- position: "bottomcenter topright",
- triggerEvent: domEvent,
- }).catch(Cu.reportError);
+ try {
+ viewShown = await PanelMultiView.openPopup(tempPanel, anchor, {
+ position: "bottomcenter topright",
+ triggerEvent: domEvent,
+ });
+ } catch (ex) {
+ Cu.reportError(ex);
+ }
+
+ if (viewShown) {
+ CustomizableUI.addPanelCloseListeners(tempPanel);
+ tempPanel.addEventListener("popuphidden", panelRemover);
+ } else {
+ panelRemover();
+ }
}
},
/**
* Sets up the event listener for when the Library view is shown.
*
* @param {panelview} viewNode The library view.
*/