Bug 1401383 - remove anchor state after transition even if the transition is canceled, and always set main view as current, r?Gijs
We weren't removing the 'open' attribute from the anchor if the transition didn't complete.
This patch fixes this by moving the addition of 'open' into _transitionViews, and its removal into
_cleanupTransitionPhase.
MozReview-Commit-ID: TS0CcwsHVN
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -427,17 +427,25 @@ this.PanelMultiView = class {
} else {
this._transitionHeight(() => {
viewNode.removeAttribute("current");
this._currentSubView = null;
this.node.setAttribute("viewtype", "main");
});
}
} else if (this.panelViews) {
- this._mainView.setAttribute("current", "true");
+ // Make sure to hide all subviews, except for the mainView.
+ let mainView = this._mainView;
+ for (let panelview of this._panelViews) {
+ if (panelview == mainView)
+ panelview.setAttribute("current", true);
+ else
+ panelview.removeAttribute("current");
+ }
+ this.node.setAttribute("viewtype", "main");
}
if (!this.panelViews) {
this._shiftMainView();
}
}
showSubView(aViewId, aAnchor, aPreviousView) {
@@ -521,31 +529,27 @@ this.PanelMultiView = class {
this._currentSubView = viewNode;
if (this.panelViews) {
if (viewNode.id == this._mainViewId) {
this.node.setAttribute("viewtype", "main");
} else {
this.node.setAttribute("viewtype", "subview");
}
+ // If we've got an older transition still running, make sure to clean it up.
+ await this._cleanupTransitionPhase();
if (!playTransition) {
viewNode.setAttribute("current", true);
this.descriptionHeightWorkaround(viewNode);
}
}
// Now we have to transition the panel.
if (this.panelViews && playTransition) {
- if (aAnchor)
- aAnchor.setAttribute("open", true);
-
- await this._transitionViews(previousViewNode, viewNode, reverse, previousRect);
-
- if (aAnchor)
- aAnchor.removeAttribute("open");
+ await this._transitionViews(previousViewNode, viewNode, reverse, previousRect, aAnchor);
this._dispatchViewEvent(viewNode, "ViewShown");
this._updateKeyboardFocus(viewNode);
} else if (!this.panelViews) {
this._transitionHeight(() => {
viewNode.setAttribute("current", true);
if (viewNode.id == this._mainViewId) {
this.node.setAttribute("viewtype", "main");
@@ -572,37 +576,39 @@ this.PanelMultiView = class {
* @param {panelview} previousViewNode Node that is currently shown as active,
* but is about to be transitioned away.
* @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 {Object} previousRect Rect object, with the same structure as
* a DOMRect, of the `previousViewNode`.
- * @param {Function} callback Function that will be invoked when the
- * transition is finished or when the
- * operation was canceled (early return).
+ * @param {Element} anchor the anchor for which we're opening
+ * a new panelview, if any
*/
- async _transitionViews(previousViewNode, viewNode, reverse, previousRect) {
+ async _transitionViews(previousViewNode, viewNode, reverse, previousRect, anchor) {
// There's absolutely no need to show off our epic animation skillz when
// the panel's not even open.
if (this._panel.state != "open") {
return;
}
const {window, document} = this;
if (this._autoResizeWorkaroundTimer)
window.clearTimeout(this._autoResizeWorkaroundTimer);
this._transitionDetails = {
phase: TRANSITION_PHASES.START,
- previousViewNode, viewNode, reverse
+ previousViewNode, viewNode, reverse, anchor
};
+ if (anchor)
+ anchor.setAttribute("open", "true");
+
// Set the viewContainer dimensions to make sure only the current view is
// visible.
this._viewContainer.style.height = Math.max(previousRect.height, this._mainViewHeight) + "px";
this._viewContainer.style.width = previousRect.width + "px";
// Lock the dimensions of the window that hosts the popup panel.
let rect = this._panel.popupBoxObject.getOuterScreenRect();
this._panel.setAttribute("width", rect.width);
this._panel.setAttribute("height", rect.height);
@@ -699,27 +705,30 @@ this.PanelMultiView = class {
* Attempt to clean up the attributes and properties set by `_transitionViews`
* above. Which attributes and properties depends on the phase the transition
* was left from - normally that'd be `TRANSITION_PHASES.END`.
*/
async _cleanupTransitionPhase() {
if (!this._transitionDetails)
return;
- let {phase, previousViewNode, viewNode, reverse, resolve, listener} = this._transitionDetails;
+ let {phase, previousViewNode, viewNode, reverse, resolve, listener, anchor} = this._transitionDetails;
this._transitionDetails = null;
// Do the things we _always_ need to do whenever the transition ends or is
// interrupted.
this._dispatchViewEvent(previousViewNode, "ViewHiding");
previousViewNode.removeAttribute("current");
if (reverse)
this._resetKeyNavigation(previousViewNode);
this.descriptionHeightWorkaround(viewNode);
+ 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(() => {
@@ -992,17 +1001,16 @@ this.PanelMultiView = class {
case "popuphidden":
// WebExtensions consumers can hide the popup from viewshowing, or
// mid-transition, which disrupts our state:
this._viewShowing = null;
this._transitioning = false;
this.node.removeAttribute("panelopen");
this.showMainView();
if (this.panelViews) {
- this._cleanupTransitionPhase();
for (let panelView of this._viewStack.children) {
if (panelView.nodeName != "children") {
panelView.__lastKnownBoundingRect = null;
panelView.style.removeProperty("min-width");
panelView.style.removeProperty("max-width");
}
}
this.window.removeEventListener("keydown", this);
--- a/browser/components/extensions/ext-browserAction.js
+++ b/browser/components/extensions/ext-browserAction.js
@@ -190,22 +190,16 @@ this.browserAction = class extends Exten
let popupURL = this.getProperty(tab, "popup");
this.tabManager.addActiveTabPermission(tab);
// Popups are shown only if a popup URL is defined; otherwise
// a "click" event is dispatched. This is done for compatibility with the
// Google Chrome onClicked extension API.
if (popupURL) {
try {
- if (event.target.closest("panelmultiview")) {
- // FIXME: The line below needs to change eventually, but for now:
- // ensure the view is _always_ visible _before_ `popup.attach()` is
- // called. PanelMultiView.jsm dictates different behavior.
- event.target.setAttribute("current", true);
- }
let popup = this.getPopup(document.defaultView, popupURL);
let attachPromise = popup.attach(event.target);
event.detail.addBlocker(attachPromise);
await attachPromise;
TelemetryStopwatch.finish(POPUP_OPEN_MS_HISTOGRAM, this);
if (this.eventQueue.length) {
let histogram = Services.telemetry.getHistogramById(POPUP_RESULT_HISTOGRAM);
histogram.add("popupShown");
--- a/browser/components/extensions/test/browser/browser-common.ini
+++ b/browser/components/extensions/test/browser/browser-common.ini
@@ -40,17 +40,16 @@ skip-if = os == 'linux'
[browser_ext_browserAction_disabled.js]
[browser_ext_browserAction_pageAction_icon.js]
[browser_ext_browserAction_pageAction_icon_permissions.js]
[browser_ext_browserAction_popup.js]
skip-if = debug && (os == 'linux' && bits == 32) # Bug 1313372
[browser_ext_browserAction_popup_preload.js]
skip-if = (os == 'win' && !debug) # bug 1352668
[browser_ext_browserAction_popup_resize.js]
-skip-if = os == 'mac' # Bug 1374749 will re-enable this test again.
[browser_ext_browserAction_simple.js]
[browser_ext_browserAction_telemetry.js]
[browser_ext_browserAction_theme_icons.js]
[browser_ext_browsingData_formData.js]
[browser_ext_browsingData_history.js]
[browser_ext_browsingData_localStorage.js]
[browser_ext_browsingData_pluginData.js]
[browser_ext_browsingData_serviceWorkers.js]
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
@@ -151,19 +151,24 @@ async function testPopupSize(standardsMo
await closeBrowserAction(extension, browserWin);
}
// Test the PanelUI panel for a menu panel button.
let widget = getBrowserActionWidget(extension);
CustomizableUI.addWidgetToArea(widget.id, getCustomizableUIPanelID());
+ let panel = browserWin.PanelUI.overflowPanel;
+ let panelMultiView = panel.firstChild;
+ let widgetId = makeWidgetId(extension.id);
+ // The 'ViewShown' event is the only way to correctly determine when the extensions'
+ // panelview has finished transitioning and is fully in view.
+ let shownPromise = BrowserTestUtils.waitForEvent(panelMultiView, "ViewShown",
+ e => (e.originalTarget.id || "").includes(widgetId));
let browser = await openPanel(extension, browserWin);
-
- let panel = browserWin.PanelUI.overflowPanel;
let origPanelRect = panel.getBoundingClientRect();
// Check that the panel is still positioned as expected.
let checkPanelPosition = () => {
is(panel.getAttribute("side"), arrowSide, "Panel arrow is positioned as expected");
let panelRect = panel.getBoundingClientRect();
if (arrowSide == "top") {
@@ -178,26 +183,20 @@ async function testPopupSize(standardsMo
ok(panelRect.top <= origPanelRect.top, `Panel has not shrunk from original size (${panelRect.top} <= ${origPanelRect.top})`);
let panelTop = browserWin.mozInnerScreenY + panelRect.top;
ok(panelTop >= browserWin.screen.availTop, `Top of popup should be on-screen. (${panelTop} >= ${browserWin.screen.availTop})`);
}
};
await awaitBrowserLoaded(browser);
-
- let panelview = browser.closest("panelview");
- // Need to wait first for the forced panel width and for the panelview's border to be gone,
- // then for layout to happen again. Otherwise the removal of the border between views in the
- // panelmultiview trips up our width checking causing it to be off-by-one.
- await BrowserTestUtils.waitForCondition(() => (!panel.hasAttribute("width") && (!panelview || !panelview.style.borderInlineStart)));
- await promiseAnimationFrame(browserWin);
+ await shownPromise;
// Wait long enough to make sure the initial resize debouncing timer has
// expired.
- await delay(100);
+ await delay(500);
let dims = await promiseContentDimensions(browser);
is(dims.isStandards, standardsMode, "Document has the expected compat mode");
// If the browser's preferred height is smaller than the initial height of the
// panel, then it will still take up the full available vertical space. Even
// so, we need to check that we've gotten the preferred height calculation