Bug 1439358 - Part 6 - Decouple view events from view visibility. r=Gijs
This stops redundant ViewHiding and late ViewShown events from being dispatched when the panel is closed during a ViewShowing event or a transition, and stops dispatching ViewHiding events when a view becomes invisible but is still open.
The panelMultiView property on "panelview" nodes is now set to null when the view is closed, indicating that the view can be immediately reused in a different panel. The Places view had to be updated so it doesn't rely on this property during the PanelMultiViewHidden event.
MozReview-Commit-ID: B1yU6si3eD3
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -624,16 +624,17 @@ this.PanelMultiView = class extends this
// The constrained width of subviews may also vary between panels.
nextPanelView.minMaxWidth = prevPanelView.knownWidth;
if (anchor) {
viewNode.classList.add("PanelUI-subView");
}
await this._transitionViews(prevPanelView.node, viewNode, false, anchor);
+ this._viewShown(nextPanelView);
}
/**
* Navigates backwards by sliding out the most recent subview.
*/
goBack() {
this._goBack().catch(Cu.reportError);
}
@@ -645,17 +646,19 @@ this.PanelMultiView = class extends this
}
let prevPanelView = this.openViews[this.openViews.length - 1];
let nextPanelView = this.openViews[this.openViews.length - 2];
prevPanelView.captureKnownSize();
await this._transitionViews(prevPanelView.node, nextPanelView.node, true);
- this.openViews.pop();
+ this._closeLatestView();
+
+ this._viewShown(nextPanelView);
}
/**
* Prepares the main view before showing the panel.
*/
async showMainView() {
if (!this.node || !this._mainViewId) {
return false;
@@ -670,16 +673,17 @@ this.PanelMultiView = class extends this
// reset all the properties that may be set on a subview.
nextPanelView.mainview = true;
nextPanelView.headerText = "";
nextPanelView.minMaxWidth = 0;
await this._cleanupTransitionPhase();
this.hideAllViewsExcept(nextPanelView);
+ this._viewShown(nextPanelView);
return true;
}
/**
* Ensures that all the panelviews, that are currently part of this instance,
* are hidden, except one specifically.
*
* @param {panelview} [nextPanelView]
@@ -722,26 +726,48 @@ this.PanelMultiView = class extends this
// point the ViewHiding event has already been dispatched for all of them.
if (!this.openViews.length) {
return false;
}
// Check if the event requested cancellation but the panel is still open.
if (canceled) {
// Handlers for ViewShowing can't know if a different handler requested
- // cancellation, so we dispatch an event to give a chance to clean up.
- panelView.dispatchCustomEvent("ViewHiding");
- this.openViews.pop();
+ // cancellation, so this will dispatch a ViewHiding event to give a chance
+ // to clean up.
+ this._closeLatestView();
return false;
}
return true;
}
/**
+ * Raises the ViewShown event if the specified view is still open.
+ */
+ _viewShown(panelView) {
+ if (panelView.node.panelMultiView == this.node) {
+ panelView.dispatchCustomEvent("ViewShown");
+ }
+ }
+
+ /**
+ * Closes the most recent PanelView and raises the ViewHiding event.
+ *
+ * @note The ViewHiding event is not cancelable and should probably be renamed
+ * to ViewHidden or ViewClosed instead, see bug 1438507.
+ */
+ _closeLatestView() {
+ let panelView = this.openViews.pop();
+ panelView.clearNavigation();
+ panelView.dispatchCustomEvent("ViewHiding");
+ panelView.node.panelMultiView = null;
+ }
+
+ /**
* 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.
*
* @param {panelview} previousViewNode Node that is currently shown as active,
* but is about to be transitioned away.
@@ -919,25 +945,22 @@ this.PanelMultiView = class extends this
if (!details || !this.node)
return;
let {phase, previousViewNode, viewNode, reverse, resolve, listener, cancelListener, anchor} = details;
if (details == this._transitionDetails)
this._transitionDetails = null;
let nextPanelView = PanelView.forNode(viewNode);
- let prevPanelView = PanelView.forNode(previousViewNode);
// Do the things we _always_ need to do whenever the transition ends or is
// interrupted.
this.hideAllViewsExcept(nextPanelView);
previousViewNode.removeAttribute("in-transition");
viewNode.removeAttribute("in-transition");
- if (reverse)
- prevPanelView.clearNavigation();
if (anchor)
anchor.removeAttribute("open");
if (phase >= TRANSITION_PHASES.START) {
this._panel.removeAttribute("width");
this._panel.removeAttribute("height");
// Myeah, panel layout auto-resizing is a funky thing. We'll wait
@@ -1062,23 +1085,24 @@ this.PanelMultiView = class extends this
// 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:
this._transitioning = false;
this.node.removeAttribute("panelopen");
- // Raise the ViewHiding event for the current view.
this._cleanupTransitionPhase();
this.hideAllViewsExcept(null);
this.window.removeEventListener("keydown", this);
this._panel.removeEventListener("mousemove", this);
- this.openViews.forEach(panelView => panelView.clearNavigation());
- this.openViews = [];
+ // Raise ViewHiding events for open views in reverse order.
+ while (this.openViews.length) {
+ this._closeLatestView();
+ }
// 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");
this._viewStack.style.removeProperty("max-height");
this._viewContainer.style.removeProperty("width");
this._viewContainer.style.removeProperty("height");
@@ -1107,20 +1131,18 @@ this.PanelView = class extends this.Asso
}
}
set current(value) {
if (value) {
if (!this.node.hasAttribute("current")) {
this.node.setAttribute("current", true);
this.descriptionHeightWorkaround();
- this.dispatchCustomEvent("ViewShown");
}
- } else if (this.node.hasAttribute("current")) {
- this.dispatchCustomEvent("ViewHiding");
+ } else {
this.node.removeAttribute("current");
}
}
/**
* Constrains the width of this view using the "min-width" and "max-width"
* styles. Setting this to zero removes the constraints.
*/
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -2112,24 +2112,16 @@ this.PlacesPanelview = class extends Pla
}
get events() {
if (this._events)
return this._events;
return this._events = ["click", "command", "dragend", "dragstart", "ViewHiding", "ViewShown"];
}
- get panel() {
- return this.panelMultiView.parentNode;
- }
-
- get panelMultiView() {
- return this._viewElt.panelMultiView;
- }
-
handleEvent(event) {
switch (event.type) {
case "click":
// For middle clicks, fall through to the command handler.
if (event.button != 1) {
break;
}
case "command":
@@ -2191,16 +2183,17 @@ this.PlacesPanelview = class extends Pla
this._controller.setDataTransfer(event);
event.stopPropagation();
}
uninit(event) {
this._removeEventListeners(this.panelMultiView, this.events);
this._removeEventListeners(window, ["unload"]);
+ delete this.panelMultiView;
super.uninit(event);
}
_createDOMNodeForPlacesNode(placesNode) {
this._domNodes.delete(placesNode);
let element;
let type = placesNode.type;
@@ -2271,16 +2264,17 @@ this.PlacesPanelview = class extends Pla
}
}
_onPopupShowing(event) {
// If the event came from the root element, this is the first time
// we ever get here.
if (event.originalTarget == this._rootElt) {
// Start listening for events from all panels inside the panelmultiview.
+ this.panelMultiView = this._viewElt.panelMultiView;
this._addEventListeners(this.panelMultiView, this.events);
}
super._onPopupShowing(event);
}
_onViewShown(event) {
if (event.originalTarget != this._viewElt)
return;