Bug 1428839 - Part 5 - Open the anchor when subview navigation starts and close it asynchronously afterwards. r=Gijs
The anchor state does not need to be cleaned up synchronously, so we can handle it seperately from the transition state.
MozReview-Commit-ID: 1CBP9OS5WmM
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -626,47 +626,59 @@ var PanelMultiView = class extends this.
if (!prevPanelView.active) {
return;
}
// Marking the view that is about to scrolled out of the visible area as
// inactive will prevent re-entrancy and also disable keyboard navigation.
// From this point onwards, "await" statements can be used safely.
prevPanelView.active = false;
- // If the ViewShowing event cancels the operation we have to re-enable
- // keyboard navigation, but this must be avoided if the panel was closed.
- if (!(await this._openView(nextPanelView))) {
- if (prevPanelView.isOpenIn(this)) {
- // We don't raise a ViewShown event because nothing actually changed.
- // Technically we should use a different state flag just because there
- // is code that could check the "active" property to determine whether
- // to wait for a ViewShown event later, but this only happens in
- // regression tests and is less likely to be a technique used in
- // production code, where use of ViewShown is less common.
- prevPanelView.active = true;
+ // Provide visual feedback while navigation is in progress, starting before
+ // the transition starts and ending when the previous view is invisible.
+ if (anchor) {
+ anchor.setAttribute("open", "true");
+ }
+ try {
+ // If the ViewShowing event cancels the operation we have to re-enable
+ // keyboard navigation, but this must be avoided if the panel was closed.
+ if (!(await this._openView(nextPanelView))) {
+ if (prevPanelView.isOpenIn(this)) {
+ // We don't raise a ViewShown event because nothing actually changed.
+ // Technically we should use a different state flag just because there
+ // is code that could check the "active" property to determine whether
+ // to wait for a ViewShown event later, but this only happens in
+ // regression tests and is less likely to be a technique used in
+ // production code, where use of ViewShown is less common.
+ prevPanelView.active = true;
+ }
+ return;
}
- return;
+
+ prevPanelView.captureKnownSize();
+
+ // The main view of a panel can be a subview in another one. Make sure to
+ // reset all the properties that may be set on a subview.
+ nextPanelView.mainview = false;
+ // The header may change based on how the subview was opened.
+ nextPanelView.headerText = viewNode.getAttribute("title") ||
+ (anchor && anchor.getAttribute("label"));
+ // 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);
+ } finally {
+ if (anchor) {
+ anchor.removeAttribute("open");
+ }
}
- prevPanelView.captureKnownSize();
-
- // The main view of a panel can be a subview in another one. Make sure to
- // reset all the properties that may be set on a subview.
- nextPanelView.mainview = false;
- // The header may change based on how the subview was opened.
- nextPanelView.headerText = viewNode.getAttribute("title") ||
- (anchor && anchor.getAttribute("label"));
- // 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._activateView(nextPanelView);
}
/**
* Navigates backwards by sliding out the most recent subview.
*/
goBack() {
this._goBack().catch(Cu.reportError);
@@ -823,39 +835,33 @@ var PanelMultiView = class extends this.
*
* @param {panelview} previousViewNode Node that is currently displayed, but
* is about to be transitioned away. This
* must be already inactive at this point.
* @param {panelview} viewNode Node that will becode the active view,
* after the transition has finished.
* @param {Boolean} reverse Whether we're navigation back to a
* previous view or forward to a next view.
- * @param {Element} anchor the anchor for which we're opening
- * a new panelview, if any
*/
- async _transitionViews(previousViewNode, viewNode, reverse, anchor) {
+ async _transitionViews(previousViewNode, viewNode, reverse) {
// Clean up any previous transition that may be active at this point.
this._cleanupTransitionPhase();
const { window } = this;
let nextPanelView = PanelView.forNode(viewNode);
let prevPanelView = PanelView.forNode(previousViewNode);
if (this._autoResizeWorkaroundTimer)
window.clearTimeout(this._autoResizeWorkaroundTimer);
let details = this._transitionDetails = {
phase: TRANSITION_PHASES.START,
- anchor
};
- if (anchor)
- anchor.setAttribute("open", "true");
-
// Set the viewContainer dimensions to make sure only the current view is
// visible.
let olderView = reverse ? nextPanelView : prevPanelView;
this._viewContainer.style.minHeight = olderView.knownHeight + "px";
this._viewContainer.style.height = prevPanelView.knownHeight + "px";
this._viewContainer.style.width = prevPanelView.knownWidth + "px";
// Lock the dimensions of the window that hosts the popup panel.
let rect = this._panel.popupBoxObject.getOuterScreenRect();
@@ -989,25 +995,20 @@ var PanelMultiView = class extends this.
* @param {Object} details Dictionary object containing details of the transition
* that should be cleaned up after. Defaults to the most
* recent details.
*/
_cleanupTransitionPhase(details = this._transitionDetails) {
if (!details || !this.node)
return;
- let {phase, resolve, listener, cancelListener, anchor} = details;
+ let {phase, resolve, listener, cancelListener} = details;
if (details == this._transitionDetails)
this._transitionDetails = null;
- // Do the things we _always_ need to do whenever the transition ends or is
- // interrupted.
- 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
// another few milliseconds to remove the width and height 'fixtures',
// to be sure we don't flicker annoyingly.
// NB: HACK! Bug 1363756 is there to fix this.
this._autoResizeWorkaroundTimer = this.window.setTimeout(() => {