Bug 1252224 - Remove synchronous layout flushes and style invalidations during the opening of multiview panels. r?mconley
MozReview-Commit-ID: 9T4gAPwFrXp
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -7223,16 +7223,18 @@ var gIdentityHandler = {
// Now open the popup, anchored off the primary chrome element
this._identityPopup.openPopup(this._identityIcon, "bottomcenter topleft");
},
onPopupShown(event) {
if (event.target == this._identityPopup) {
window.addEventListener("focus", this, true);
}
+ this._identityPopupMultiView._mainView.style.height =
+ this._identityPopup.getBoundingClientRect().height + "px";
},
onPopupHidden(event) {
if (event.target == this._identityPopup) {
window.removeEventListener("focus", this, true);
this._identityBox.removeAttribute("open");
}
},
--- a/browser/components/customizableui/content/panelUI.xml
+++ b/browser/components/customizableui/content/panelUI.xml
@@ -53,43 +53,26 @@
<field name="_panel" readonly="true">
this.parentNode;
</field>
<field name="_currentSubView">null</field>
<field name="_anchorElement">null</field>
<field name="_mainViewHeight">0</field>
<field name="_subViewObserver">null</field>
- <field name="__transitioning">false</field>
- <field name="_ignoreMutations">false</field>
+ <field name="__transitioning">true</field>
<property name="showingSubView" readonly="true"
onget="return this._viewStack.getAttribute('viewtype') == 'subview'"/>
<property name="_mainViewId" onget="return this.getAttribute('mainViewId');" onset="this.setAttribute('mainViewId', val); return val;"/>
<property name="_mainView" readonly="true"
onget="return this._mainViewId ? document.getElementById(this._mainViewId) : null;"/>
<property name="showingSubViewAsMainView" readonly="true"
onget="return this.getAttribute('mainViewIsSubView') == 'true'"/>
- <property name="ignoreMutations">
- <getter>
- return this._ignoreMutations;
- </getter>
- <setter><![CDATA[
- this._ignoreMutations = val;
- if (!val && this._panel.state == "open") {
- if (this.showingSubView) {
- this._syncContainerWithSubView();
- } else {
- this._syncContainerWithMainView();
- }
- }
- ]]></setter>
- </property>
-
<property name="_transitioning">
<getter>
return this.__transitioning;
</getter>
<setter><![CDATA[
this.__transitioning = val;
if (val) {
this.setAttribute("transitioning", "true");
@@ -218,17 +201,20 @@
</method>
<method name="_setViewContainerHeight">
<parameter name="aHeight"/>
<body><![CDATA[
let container = this._viewContainer;
this._transitioning = true;
- let onTransitionEnd = () => {
+ let onTransitionEnd = (event) => {
+ if (event.propertyName != "transform") {
+ return;
+ }
container.removeEventListener("transitionend", onTransitionEnd);
this._transitioning = false;
};
container.addEventListener("transitionend", onTransitionEnd);
container.style.height = `${aHeight}px`;
]]></body>
</method>
@@ -291,35 +277,43 @@
if (this.showingSubView) {
setTimeout(this._syncContainerWithSubView.bind(this), 0);
} else if (!this.transitioning) {
setTimeout(this._syncContainerWithMainView.bind(this), 0);
}
}
break;
case "popupshowing":
- this.setAttribute("panelopen", "true");
// Bug 941196 - The panel can get taller when opening a subview. Disabling
// autoPositioning means that the panel won't jump around if an opened
// subview causes the panel to exceed the dimensions of the screen in the
// direction that the panel originally opened in. This property resets
// every time the popup closes, which is why we have to set it each time.
this._panel.autoPosition = false;
this._syncContainerWithMainView();
this._mainViewObserver.observe(this._mainView, {
attributes: true,
characterData: true,
childList: true,
subtree: true
});
- break;
- case "popupshown":
- this._setMaxHeight();
+ let onTransitionEnd = (event) => {
+ if (event.propertyName != "transform") {
+ return;
+ }
+ let panel = event.target;
+ panel.removeEventListener("tranitionend", onTransitionEnd);
+ // Needed in case the panel is closed before the transition ends.
+ if (panel.state == "open") {
+ this.setAttribute("panelopen", "true");
+ }
+ };
+ this._panel.addEventListener("transitionend", onTransitionEnd);
break;
case "popuphidden":
this.removeAttribute("panelopen");
this._mainView.style.removeProperty("height");
this.showMainView();
this._mainViewObserver.disconnect();
break;
}
@@ -333,32 +327,19 @@
</method>
<method name="_shouldSetHeight">
<body><![CDATA[
return this.getAttribute("nosubviews") != "true";
]]></body>
</method>
- <method name="_setMaxHeight">
- <body><![CDATA[
- if (!this._shouldSetHeight())
- return;
-
- // Ignore the mutation that'll fire when we set the height of
- // the main view.
- this.ignoreMutations = true;
- this._mainView.style.height =
- this.getBoundingClientRect().height + "px";
- this.ignoreMutations = false;
- ]]></body>
- </method>
<method name="_adjustContainerHeight">
<body><![CDATA[
- if (!this.ignoreMutations && !this.showingSubView && !this._transitioning) {
+ if (!this.showingSubView && !this._transitioning) {
let height;
if (this.showingSubViewAsMainView) {
height = this._heightOfSubview(this._mainView);
} else {
height = this._mainView.scrollHeight;
}
this._viewContainer.style.height = height + "px";
}
@@ -366,17 +347,17 @@
</method>
<method name="_syncContainerWithSubView">
<body><![CDATA[
// Check that this panel is still alive:
if (!this._panel || !this._panel.parentNode) {
return;
}
- if (!this.ignoreMutations && this.showingSubView) {
+ if (this.showingSubView) {
let newHeight = this._heightOfSubview(this._currentSubView, this._subViews);
this._viewContainer.style.height = newHeight + "px";
}
]]></body>
</method>
<method name="_syncContainerWithMainView">
<body><![CDATA[
// Check that this panel is still alive:
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -144,12 +144,13 @@ skip-if = os == "linux" # crashing on Li
[browser_1087303_button_fullscreen.js]
tags = fullscreen
skip-if = os == "mac"
[browser_1087303_button_preferences.js]
[browser_1089591_still_customizable_after_reset.js]
[browser_1096763_seen_widgets_post_reset.js]
[browser_1161838_inserted_new_default_buttons.js]
[browser_bootstrapped_custom_toolbar.js]
+[browser_check_tooltips_in_navbar.js]
[browser_customizemode_contextmenu_menubuttonstate.js]
+[browser_no_mutationrecords_during_panel_opening.js]
[browser_panel_toggle.js]
[browser_switch_to_customize_mode.js]
-[browser_check_tooltips_in_navbar.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_no_mutationrecords_during_panel_opening.js
@@ -0,0 +1,79 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+/**
+ * Test that we don't get unexpected mutations during the opening of the
+ * browser menu.
+ */
+
+add_task(function* test_setup() {
+ yield resetCustomization();
+ yield PanelUI.show();
+ let hiddenPromise = promisePanelHidden(window);
+ PanelUI.hide();
+ yield hiddenPromise;
+});
+
+add_task(function* no_mutation_events_during_opening() {
+ let panel = PanelUI.panel;
+ yield PanelUI.ensureReady();
+
+ let failures = 0;
+ let observer = new MutationObserver(function(mutations) {
+ for (let mutation of mutations) {
+ if (mutation.target.localName == "panel" &&
+ mutation.type == "attributes" &&
+ mutation.attributeName == "animate") {
+ // This mutation is allowed because it triggers the CSS transition.
+ continue;
+ }
+
+ if (mutation.type == "attributes" &&
+ mutation.attributeName == "panelopen") {
+ // This mutation is allowed because it is set after the panel has
+ // finished the transition.
+ continue;
+ }
+
+ let newValue = null;
+ if (mutation.type == "attributes") {
+ newValue = mutation.target.getAttribute(mutation.attributeName);
+ } else if (mutation.type == "characterData") {
+ newValue = mutation.target.textContent;
+ }
+
+ if (newValue == mutation.oldValue) {
+ // Mutations records are observed even when the new and old value are
+ // identical. This is unlikely to invalidate the panel, so ignore these.
+ continue;
+ }
+
+ let nodeIdentifier = `${mutation.target.localName}#${mutation.target.id}.${mutation.target.className};`;
+ ok(false, `Observed: ${mutation.type}; ${nodeIdentifier} ${mutation.attributeName}; oldValue: ${mutation.oldValue}; newValue: ${newValue}`);
+ failures++;
+ }
+ });
+ observer.observe(panel, {
+ childList: true,
+ attributes: true,
+ characterData: true,
+ subtree: true,
+ attributeOldValue: true,
+ characterDataOldValue: true,
+ });
+ let shownPromise = promisePanelShown(window);
+ PanelUI.show();
+ yield shownPromise;
+ observer.disconnect();
+
+ is(failures, 0, "There should be no unexpected mutation events during opening of the panel");
+});
+
+add_task(function* cleanup() {
+ let hiddenPromise = promisePanelHidden(window);
+ PanelUI.hide();
+ yield hiddenPromise;
+});
--- a/toolkit/content/widgets/popup.xml
+++ b/toolkit/content/widgets/popup.xml
@@ -420,17 +420,19 @@
if (position.indexOf("start_") == 0 || position.indexOf("end_") == 0) {
container.orient = "horizontal";
arrowbox.orient = "vertical";
if (position.indexOf("_after") > 0) {
arrowbox.pack = "end";
} else {
arrowbox.pack = "start";
}
- arrowbox.style.transform = "translate(0, " + -offset + "px)";
+ if (offset != "0") {
+ arrowbox.style.transform = "translate(0, " + -offset + "px)";
+ }
// The assigned side stays the same regardless of direction.
var isRTL = (window.getComputedStyle(this).direction == "rtl");
if (position.indexOf("start_") == 0) {
container.dir = "reverse";
this.setAttribute("side", isRTL ? "left" : "right");
}
@@ -442,17 +444,19 @@
else if (position.indexOf("before_") == 0 || position.indexOf("after_") == 0) {
container.orient = "";
arrowbox.orient = "";
if (position.indexOf("_end") > 0) {
arrowbox.pack = "end";
} else {
arrowbox.pack = "start";
}
- arrowbox.style.transform = "translate(" + -offset + "px, 0)";
+ if (offset != "0") {
+ arrowbox.style.transform = "translate(" + -offset + "px, 0)";
+ }
if (position.indexOf("before_") == 0) {
container.dir = "reverse";
this.setAttribute("side", "bottom");
}
else {
container.dir = "";
this.setAttribute("side", "top");