Bug 1447619 - avoid reflowing when returning items to the toolbar from the overflow menu, r=florian
MozReview-Commit-ID: BFGRssWb9F
--- a/browser/base/content/test/performance/browser_window_resize.js
+++ b/browser/base/content/test/performance/browser_window_resize.js
@@ -8,37 +8,19 @@
* is a whitelist that should slowly go away as we improve the performance of
* the front-end. Instead of adding more reflows to the whitelist, you should
* be modifying your code to avoid the reflow.
*
* See https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers
* 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");
/**
* Sets the visibility state on the Bookmarks Toolbar, and
* waits for it to transition to fully visible.
*
--- a/browser/components/customizableui/CustomizableUI.jsm
+++ b/browser/components/customizableui/CustomizableUI.jsm
@@ -4403,28 +4403,46 @@ OverflowableToolbar.prototype = {
doc.defaultView.updateEditUIVisibility();
let contextMenuId = this._panel.getAttribute("context");
if (contextMenuId) {
let contextMenu = doc.getElementById(contextMenuId);
gELS.removeSystemEventListener(contextMenu, "command", this, true);
}
},
- 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") {
this._collapsed.set(child.id, this._target.clientWidth);
child.setAttribute("overflowedItem", true);
child.setAttribute("cui-anchorid", this._chevron.id);
CustomizableUIInternal.ensureButtonContextMenu(child, this._toolbar, true);
CustomizableUIInternal.notifyListeners("onWidgetOverflow", child, this._target);
@@ -4433,40 +4451,70 @@ OverflowableToolbar.prototype = {
if (!this._addedListener) {
CustomizableUI.addListener(this);
}
if (!CustomizableUI.isSpecialWidget(child.id)) {
this._toolbar.setAttribute("overflowing", "true");
}
}
child = prevChild;
- }
-
- let win = this._target.ownerGlobal;
+ [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;
+ }
+ }
+
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);
}
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);
let beforeNodeIndex = placements.indexOf(child.id) + 1;
// If this is a skipintoolbarset item, meaning it doesn't occur in the placements list,
// we're inserting it at the end. This will mean first-in, first-out (more or less)
// leading to as little change in order as possible.
if (beforeNodeIndex == 0) {
@@ -4488,37 +4536,43 @@ OverflowableToolbar.prototype = {
this._target.appendChild(child);
}
child.removeAttribute("cui-anchorid");
child.removeAttribute("overflowedItem");
CustomizableUIInternal.ensureButtonContextMenu(child, this._target);
CustomizableUIInternal.notifyListeners("onWidgetUnderflow", child, this._target);
}
- let win = this._target.ownerGlobal;
win.UpdateUrlbarSearchSplitterState();
let collapsedWidgetIds = Array.from(this._collapsed.keys());
if (collapsedWidgetIds.every(w => CustomizableUI.isSpecialWidget(w))) {
this._toolbar.removeAttribute("overflowing");
}
if (this._addedListener && !this._collapsed.size) {
CustomizableUI.removeListener(this);
this._addedListener = false;
}
},
- _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);
}
},
_disable() {
this._enabled = false;
this._moveItemsBackToTheirOrigin(true);
if (this._lazyResizeHandler) {
this._lazyResizeHandler.disarm();
@@ -4603,17 +4657,17 @@ OverflowableToolbar.prototype = {
} else if (aNode.previousSibling) {
// but if it still is, it must have changed places. Bookkeep:
let prevId = aNode.previousSibling.id;
let minSize = this._collapsed.get(prevId);
this._collapsed.set(aNode.id, minSize);
} else {
// If it's now the first item in the overflow list,
// maybe we can return it:
- this._moveItemsBackToTheirOrigin();
+ this._moveItemsBackToTheirOrigin(false);
}
},
findOverflowedInsertionPoints(aNode) {
let newNodeCanOverflow = aNode.getAttribute("overflows") != "false";
let areaId = this._toolbar.id;
let placements = gPlacements.get(areaId);
let nodeIndex = placements.indexOf(aNode.id);
--- a/browser/components/customizableui/test/browser_901207_searchbar_in_panel.js
+++ b/browser/components/customizableui/test/browser_901207_searchbar_in_panel.js
@@ -44,26 +44,29 @@ add_task(async function() {
let hiddenPanelPromise = promiseOverflowHidden(window);
EventUtils.synthesizeKey("KEY_Escape");
await hiddenPanelPromise;
CustomizableUI.reset();
});
// Ctrl+K should open the overflow panel and focus the search bar if the search bar is overflowed.
-add_task(async function() {
+add_task(async function check_shortcut_when_in_overflow() {
this.originalWindowWidth = window.outerWidth;
let navbar = document.getElementById(CustomizableUI.AREA_NAVBAR);
ok(!navbar.hasAttribute("overflowing"), "Should start with a non-overflowing toolbar.");
ok(CustomizableUI.inDefaultState, "Should start in default state.");
Services.prefs.setBoolPref("browser.search.widget.inNavBar", true);
window.resizeTo(kForceOverflowWidthPx, window.outerHeight);
- await waitForCondition(() => navbar.getAttribute("overflowing") == "true");
+ await waitForCondition(() => {
+ return navbar.getAttribute("overflowing") == "true" &&
+ !navbar.querySelector("#search-container");
+ });
ok(!navbar.querySelector("#search-container"), "Search container should be overflowing");
let shownPanelPromise = promiseOverflowShown(window);
sendWebSearchKeyCommand();
await shownPanelPromise;
let chevron = document.getElementById("nav-bar-overflow-button");
await waitForCondition(() => chevron.open);
--- a/browser/components/customizableui/test/browser_976792_insertNodeInWindow.js
+++ b/browser/components/customizableui/test/browser_976792_insertNodeInWindow.js
@@ -63,17 +63,17 @@ add_task(async function() {
}
for (let id of widgetIds) {
document.getElementById(id).style.minWidth = "200px";
}
let originalWindowWidth = window.outerWidth;
window.resizeTo(kForceOverflowWidthPx, window.outerHeight);
- await waitForCondition(() => navbar.hasAttribute("overflowing"));
+ await waitForCondition(() => navbar.hasAttribute("overflowing") && !navbar.querySelector("#" + widgetIds[0]));
let testWidgetId = kTestWidgetPrefix + 3;
CustomizableUI.destroyWidget(testWidgetId);
let btn = createDummyXULButton(testWidgetId, "test");
CustomizableUI.ensureWidgetPlacedInWindow(testWidgetId, window);
@@ -112,17 +112,17 @@ add_task(async function() {
}
for (let id of widgetIds) {
document.getElementById(id).style.minWidth = "200px";
}
let originalWindowWidth = window.outerWidth;
window.resizeTo(kForceOverflowWidthPx, window.outerHeight);
- await waitForCondition(() => navbar.hasAttribute("overflowing"));
+ await waitForCondition(() => navbar.hasAttribute("overflowing") && !navbar.querySelector("#" + widgetIds[0]));
let testWidgetId = kTestWidgetPrefix + 3;
CustomizableUI.destroyWidget(kTestWidgetPrefix + 2);
CustomizableUI.destroyWidget(testWidgetId);
let btn = createDummyXULButton(testWidgetId, "test");
CustomizableUI.ensureWidgetPlacedInWindow(testWidgetId, window);
@@ -162,17 +162,17 @@ add_task(async function() {
}
for (let id of widgetIds) {
document.getElementById(id).style.minWidth = "200px";
}
let originalWindowWidth = window.outerWidth;
window.resizeTo(kForceOverflowWidthPx, window.outerHeight);
- await waitForCondition(() => navbar.hasAttribute("overflowing"));
+ await waitForCondition(() => navbar.hasAttribute("overflowing") && !navbar.querySelector("#" + widgetIds[0]));
let testWidgetId = kTestWidgetPrefix + 3;
CustomizableUI.destroyWidget(kTestWidgetPrefix + 2);
CustomizableUI.destroyWidget(testWidgetId);
CustomizableUI.destroyWidget(kTestWidgetPrefix + 4);
let btn = createDummyXULButton(testWidgetId, "test");
@@ -221,17 +221,24 @@ add_task(async function() {
}
for (let id of widgetIds) {
document.getElementById(id).style.minWidth = "200px";
}
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;
is(nonOverflowing.getAttribute("overflows"), "false", "Last child is expected to not allow overflowing");
isnot(nonOverflowing.getAttribute("skipintoolbarset"), "true", "Last child is expected to not be skipintoolbarset");
let testWidgetId = kTestWidgetPrefix + 10;
CustomizableUI.destroyWidget(testWidgetId);
@@ -282,17 +289,17 @@ add_task(async function() {
}
let toolbarNode = createOverflowableToolbarWithPlacements(kToolbarName, widgetIds);
assertAreaPlacements(kToolbarName, widgetIds);
ok(!toolbarNode.hasAttribute("overflowing"), "Toolbar shouldn't overflow to start with.");
let originalWindowWidth = window.outerWidth;
window.resizeTo(kForceOverflowWidthPx, window.outerHeight);
- await waitForCondition(() => toolbarNode.hasAttribute("overflowing"));
+ await waitForCondition(() => toolbarNode.hasAttribute("overflowing") && !toolbarNode.querySelector("#" + widgetIds[1]));
ok(toolbarNode.hasAttribute("overflowing"), "Should have an overflowing toolbar.");
let btnId = kTestWidgetPrefix + missingId;
let btn = createDummyXULButton(btnId, "test");
CustomizableUI.ensureWidgetPlacedInWindow(btnId, window);
is(btn.parentNode.id, kToolbarName + "-overflow-list", "New XUL widget should be placed inside new toolbar's overflow");
is(btn.previousSibling.id, kTestWidgetPrefix + 1,
--- a/browser/components/customizableui/test/browser_editcontrols_update.js
+++ b/browser/components/customizableui/test/browser_editcontrols_update.js
@@ -86,17 +86,17 @@ add_task(async function test_panelui_ope
await overridePromise;
checkState(true, "Update when edit-controls is on panel, hidden and selection changed");
});
// Test updating when the edit-controls are moved to the toolbar.
add_task(async function test_panelui_customize_to_toolbar() {
await startCustomizing();
let navbar = document.getElementById("nav-bar");
- simulateItemDrag(document.getElementById("edit-controls"), navbar.customizationTarget);
+ simulateItemDrag(document.getElementById("edit-controls"), navbar.customizationTarget, "end");
await endCustomizing();
// updateEditUIVisibility should be called when customization ends but isn't. See bug 1359790.
updateEditUIVisibility();
// The URL bar may have been focused to begin with, which means
// that subsequent calls to focus it won't result in command
// updates, so we'll make sure to blur it.
@@ -120,17 +120,18 @@ add_task(async function test_panelui_cus
registerCleanupFunction(async function() {
kOverflowPanel.removeAttribute("animate");
window.resizeTo(originalWidth, window.outerHeight);
await waitForCondition(() => !navbar.hasAttribute("overflowing"));
CustomizableUI.reset();
});
window.resizeTo(kForceOverflowWidthPx, window.outerHeight);
- await waitForCondition(() => navbar.hasAttribute("overflowing"));
+ await waitForCondition(() =>
+ navbar.hasAttribute("overflowing") && !navbar.querySelector("edit-controls"));
// Mac will update the enabled state even when the buttons are overflowing,
// so main menubar shortcuts will work properly.
overridePromise = expectCommandUpdate(isMac ? 1 : 0);
gURLBar.select();
await overridePromise;
checkState(true, "Update when edit-controls is on overflow panel, hidden and selection changed");
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -1165,21 +1165,25 @@ PlacesToolbar.prototype = {
// the overflow status of the toolbar.
if (aEvent.target == aEvent.currentTarget) {
this.updateNodesVisibility();
}
break;
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":
case "TabClose":
this.updateNodesVisibility();
break;
case "dragstart":
this._onDragStart(aEvent);