diff --git a/browser/base/content/test/performance/browser_window_resize.js b/browser/base/content/test/performance/browser_window_resize.js index 0c64dafc66f8..b9ec9dd49485 100644 --- a/browser/base/content/test/performance/browser_window_resize.js +++ b/browser/base/content/test/performance/browser_window_resize.js @@ -13,27 +13,9 @@ * for tips on how to do that. */ const EXPECTED_REFLOWS = [ - { - stack: [ - "onOverflow@resource:///modules/CustomizableUI.jsm", - ], - maxCount: 48, - }, - - { - stack: [ - "_moveItemsBackToTheirOrigin@resource:///modules/CustomizableUI.jsm", - "_onLazyResize@resource:///modules/CustomizableUI.jsm", - ], - maxCount: 5, - }, - - { - stack: [ - "_onLazyResize@resource:///modules/CustomizableUI.jsm", - ], - maxCount: 4, - }, + /** + * Nothing here! Please don't add anything new! + */ ]; const gToolbar = document.getElementById("PersonalToolbar"); diff --git a/browser/components/customizableui/CustomizableUI.jsm b/browser/components/customizableui/CustomizableUI.jsm index 9f34799e4791..8cd4d6481313 100644 --- a/browser/components/customizableui/CustomizableUI.jsm +++ b/browser/components/customizableui/CustomizableUI.jsm @@ -4408,18 +4408,36 @@ OverflowableToolbar.prototype = { } }, - onOverflow(aEvent) { - // The rangeParent check is here because of bug 1111986 and ensuring that - // overflow events from the bookmarks toolbar items or similar things that - // manage their own overflow don't trigger an overflow on the entire toolbar - if (!this._enabled || - (aEvent && aEvent.target != this._toolbar.customizationTarget) || - (aEvent && aEvent.rangeParent)) + /** + * Avoid re-entrancy in the overflow handling by keeping track of invocations: + */ + _lastOverflowCounter: 0, + + /** + * Handle overflow in the toolbar by moving items to the overflow menu. + * @param {Event} aEvent + * The overflow event that triggered handling overflow. May be omitted + * in some cases (e.g. when we run this method after overflow handling + * is re-enabled from customize mode, to ensure correct handling of + * initial overflow). + */ + async onOverflow(aEvent) { + if (!this._enabled) return; let child = this._target.lastChild; - while (child && this._target.scrollLeftMin != this._target.scrollLeftMax) { + let thisOverflowResponse = ++this._lastOverflowCounter; + + let win = this._target.ownerGlobal; + let [scrollLeftMin, scrollLeftMax] = await win.promiseDocumentFlushed(() => { + return [this._target.scrollLeftMin, this._target.scrollLeftMax]; + }); + if (win.closed || this._lastOverflowCounter != thisOverflowResponse) { + return; + } + + while (child && scrollLeftMin != scrollLeftMax) { let prevChild = child.previousSibling; if (child.getAttribute("overflows") != "false") { @@ -4438,13 +4456,26 @@ OverflowableToolbar.prototype = { } } child = prevChild; + [scrollLeftMin, scrollLeftMax] = await win.promiseDocumentFlushed(() => { + return [this._target.scrollLeftMin, this._target.scrollLeftMax]; + }); + // If the window has closed or if we re-enter because we were waiting + // for layout, stop. + if (win.closed || this._lastOverflowCounter != thisOverflowResponse) { + return; + } } - let win = this._target.ownerGlobal; win.UpdateUrlbarSearchSplitterState(); + // Reset the counter because we finished handling overflow. + this._lastOverflowCounter = 0; }, _onResize(aEvent) { + // Ignore bubbled-up resize events. + if (aEvent.target != aEvent.target.ownerGlobal.top) { + return; + } if (!this._lazyResizeHandler) { this._lazyResizeHandler = new DeferredTask(this._onLazyResize.bind(this), LAZY_RESIZE_INTERVAL_MS, 0); @@ -4452,16 +4483,33 @@ OverflowableToolbar.prototype = { this._lazyResizeHandler.arm(); }, - _moveItemsBackToTheirOrigin(shouldMoveAllItems) { + /** + * Try to move toolbar items back to the toolbar from the overflow menu. + * @param {boolean} shouldMoveAllItems + * Whether we should move everything (e.g. because we're being disabled) + * @param {number} targetWidth + * Optional; the width of the toolbar in which we can put things. + * Some consumers pass this to avoid reflows. + * While there are items in the list, this width won't change, and so + * we can avoid flushing layout by providing it and/or caching it. + * Note that if `shouldMoveAllItems` is true, we never need the width + * anyway. + */ + _moveItemsBackToTheirOrigin(shouldMoveAllItems, targetWidth) { let placements = gPlacements.get(this._toolbar.id); + let win = this._target.ownerGlobal; while (this._list.firstChild) { let child = this._list.firstChild; let minSize = this._collapsed.get(child.id); - if (!shouldMoveAllItems && - minSize && - this._target.clientWidth <= minSize) { - break; + if (!shouldMoveAllItems && minSize) { + if (!targetWidth) { + let dwu = win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils); + targetWidth = Math.floor(dwu.getBoundsWithoutFlushing(this._target).width); + } + if (targetWidth <= minSize) { + break; + } } this._collapsed.delete(child.id); @@ -4493,7 +4541,6 @@ OverflowableToolbar.prototype = { CustomizableUIInternal.notifyListeners("onWidgetUnderflow", child, this._target); } - let win = this._target.ownerGlobal; win.UpdateUrlbarSearchSplitterState(); let collapsedWidgetIds = Array.from(this._collapsed.keys()); @@ -4506,14 +4553,21 @@ OverflowableToolbar.prototype = { } }, - _onLazyResize() { + async _onLazyResize() { if (!this._enabled) return; - if (this._target.scrollLeftMin != this._target.scrollLeftMax) { + let win = this._target.ownerGlobal; + let [min, max, targetWidth] = await win.promiseDocumentFlushed(() => { + return [this._target.scrollLeftMin, this._target.scrollLeftMax, this._target.clientWidth]; + }); + if (win.closed) { + return; + } + if (min != max) { this.onOverflow(); } else { - this._moveItemsBackToTheirOrigin(); + this._moveItemsBackToTheirOrigin(false, targetWidth); } }, @@ -4608,7 +4662,7 @@ OverflowableToolbar.prototype = { } else { // If it's now the first item in the overflow list, // maybe we can return it: - this._moveItemsBackToTheirOrigin(); + this._moveItemsBackToTheirOrigin(false); } }, diff --git a/browser/components/customizableui/test/browser_976792_insertNodeInWindow.js b/browser/components/customizableui/test/browser_976792_insertNodeInWindow.js index 5fdf041ba4d9..8319e9de5ae3 100644 --- a/browser/components/customizableui/test/browser_976792_insertNodeInWindow.js +++ b/browser/components/customizableui/test/browser_976792_insertNodeInWindow.js @@ -226,7 +226,14 @@ add_task(async function() { let originalWindowWidth = window.outerWidth; window.resizeTo(kForceOverflowWidthPx, window.outerHeight); - await waitForCondition(() => navbar.hasAttribute("overflowing")); + // Wait for all the widgets to overflow. We can't just wait for the + // `overflowing` attribute because we leave time for layout flushes + // inbetween, so it's possible for the timeout to run before the + // navbar has "settled" + await waitForCondition(() => { + return navbar.hasAttribute("overflowing") && + navbar.customizationTarget.lastChild.getAttribute("overflows") == "false"; + }); // Find last widget that doesn't allow overflowing let nonOverflowing = navbar.customizationTarget.lastChild; diff --git a/browser/components/places/content/browserPlacesViews.js b/browser/components/places/content/browserPlacesViews.js index 4ac9df297769..f3cbac8cc226 100644 --- a/browser/components/places/content/browserPlacesViews.js +++ b/browser/components/places/content/browserPlacesViews.js @@ -1172,11 +1172,15 @@ PlacesToolbar.prototype = { case "overflow": if (!this._isOverflowStateEventRelevant(aEvent)) return; + // Avoid triggering overflow in containers if possible + aEvent.stopPropagation(); this._onOverflow(); break; case "underflow": if (!this._isOverflowStateEventRelevant(aEvent)) return; + // Avoid triggering underflow in containers if possible + aEvent.stopPropagation(); this._onUnderflow(); break; case "TabOpen":