Bug 1402272 - Replace adjustSiteIconStart with padding on all richlistitems. r?mak
MozReview-Commit-ID: 8JyJWLL0nJU
--- a/browser/base/content/test/performance/browser_urlbar_keyed_search_reflows.js
+++ b/browser/base/content/test/performance/browser_urlbar_keyed_search_reflows.js
@@ -11,28 +11,16 @@ requestLongerTimeout(5);
* 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.
*/
/* These reflows happen only the first time the awesomebar panel opens. */
const EXPECTED_REFLOWS_FIRST_OPEN = [
- // Bug 1357054
- {
- stack: [
- "_handleOverflow@chrome://global/content/bindings/autocomplete.xml",
- "handleOverUnderflow@chrome://global/content/bindings/autocomplete.xml",
- "_onChanged@chrome://global/content/bindings/autocomplete.xml",
- "_appendCurrentResult/<@chrome://global/content/bindings/autocomplete.xml",
- ],
- times: 18, // This number should only ever go down - never up.
- minTimes: 12,
- },
-
{
stack: [
"_rebuild@chrome://browser/content/search/search.xml",
"set_popup@chrome://browser/content/search/search.xml",
"enableOneOffSearches@chrome://browser/content/urlbarBindings.xml",
"_enableOrDisableOneOffSearches@chrome://browser/content/urlbarBindings.xml",
"urlbar_XBL_Constructor/<@chrome://browser/content/urlbarBindings.xml",
"openPopup@chrome://global/content/bindings/popup.xml",
@@ -49,25 +37,16 @@ const EXPECTED_REFLOWS_FIRST_OPEN = [
"adjustHeight@chrome://global/content/bindings/autocomplete.xml",
"onxblpopupshown@chrome://global/content/bindings/autocomplete.xml"
],
times: 5, // This number should only ever go down - never up.
},
{
stack: [
- "_handleOverflow@chrome://global/content/bindings/autocomplete.xml",
- "handleOverUnderflow@chrome://global/content/bindings/autocomplete.xml",
- "set_siteIconStart@chrome://global/content/bindings/autocomplete.xml",
- ],
- times: 6, // This number should only ever go down - never up.
- },
-
- {
- stack: [
"adjustHeight@chrome://global/content/bindings/autocomplete.xml",
"_invalidate/this._adjustHeightTimeout<@chrome://global/content/bindings/autocomplete.xml",
],
times: 51, // This number should only ever go down - never up.
},
{
stack: [
@@ -76,26 +55,16 @@ const EXPECTED_REFLOWS_FIRST_OPEN = [
"_reuseAcItem@chrome://global/content/bindings/autocomplete.xml",
"_appendCurrentResult@chrome://global/content/bindings/autocomplete.xml",
"_invalidate@chrome://global/content/bindings/autocomplete.xml",
"invalidate@chrome://global/content/bindings/autocomplete.xml"
],
times: 60, // This number should only ever go down - never up.
},
- {
- stack: [
- "_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
- "openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
- "openPopup@chrome://global/content/bindings/autocomplete.xml",
- "set_popupOpen@chrome://global/content/bindings/autocomplete.xml",
- ],
- times: 3, // This number should only ever go down - never up.
- },
-
// Bug 1359989
{
stack: [
"openPopup@chrome://global/content/bindings/popup.xml",
"_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openPopup@chrome://global/content/bindings/autocomplete.xml",
"set_popupOpen@chrome://global/content/bindings/autocomplete.xml",
--- a/browser/base/content/test/performance/browser_urlbar_search_reflows.js
+++ b/browser/base/content/test/performance/browser_urlbar_search_reflows.js
@@ -11,28 +11,16 @@ requestLongerTimeout(5);
* 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.
*/
/* These reflows happen only the first time the awesomebar panel opens. */
const EXPECTED_REFLOWS_FIRST_OPEN = [
- // Bug 1357054
- {
- stack: [
- "_handleOverflow@chrome://global/content/bindings/autocomplete.xml",
- "handleOverUnderflow@chrome://global/content/bindings/autocomplete.xml",
- "_onChanged@chrome://global/content/bindings/autocomplete.xml",
- "_appendCurrentResult/<@chrome://global/content/bindings/autocomplete.xml",
- ],
- times: 6, // This number should only ever go down - never up.
- minTimes: 0, // Sometimes this is not hit.
- },
-
{
stack: [
"_rebuild@chrome://browser/content/search/search.xml",
"set_popup@chrome://browser/content/search/search.xml",
"enableOneOffSearches@chrome://browser/content/urlbarBindings.xml",
"_enableOrDisableOneOffSearches@chrome://browser/content/urlbarBindings.xml",
"urlbar_XBL_Constructor/<@chrome://browser/content/urlbarBindings.xml",
"openPopup@chrome://global/content/bindings/popup.xml",
@@ -49,25 +37,16 @@ const EXPECTED_REFLOWS_FIRST_OPEN = [
"adjustHeight@chrome://global/content/bindings/autocomplete.xml",
"onxblpopupshown@chrome://global/content/bindings/autocomplete.xml"
],
times: 5, // This number should only ever go down - never up.
},
{
stack: [
- "_handleOverflow@chrome://global/content/bindings/autocomplete.xml",
- "handleOverUnderflow@chrome://global/content/bindings/autocomplete.xml",
- "set_siteIconStart@chrome://global/content/bindings/autocomplete.xml",
- ],
- times: 6, // This number should only ever go down - never up.
- },
-
- {
- stack: [
"adjustHeight@chrome://global/content/bindings/autocomplete.xml",
"_invalidate/this._adjustHeightTimeout<@chrome://global/content/bindings/autocomplete.xml",
],
times: 3, // This number should only ever go down - never up.
},
{
stack: [
@@ -76,26 +55,16 @@ const EXPECTED_REFLOWS_FIRST_OPEN = [
"_reuseAcItem@chrome://global/content/bindings/autocomplete.xml",
"_appendCurrentResult@chrome://global/content/bindings/autocomplete.xml",
"_invalidate@chrome://global/content/bindings/autocomplete.xml",
"invalidate@chrome://global/content/bindings/autocomplete.xml"
],
times: 60, // This number should only ever go down - never up.
},
- {
- stack: [
- "_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
- "openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
- "openPopup@chrome://global/content/bindings/autocomplete.xml",
- "set_popupOpen@chrome://global/content/bindings/autocomplete.xml",
- ],
- times: 3, // This number should only ever go down - never up.
- },
-
// Bug 1359989
{
stack: [
"openPopup@chrome://global/content/bindings/popup.xml",
"_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openPopup@chrome://global/content/bindings/autocomplete.xml",
"set_popupOpen@chrome://global/content/bindings/autocomplete.xml",
@@ -116,27 +85,16 @@ const EXPECTED_REFLOWS_SECOND_OPEN = [
{
stack: [
"adjustHeight@chrome://global/content/bindings/autocomplete.xml",
"_invalidate/this._adjustHeightTimeout<@chrome://global/content/bindings/autocomplete.xml",
],
times: 3, // This number should only ever go down - never up.
},
- // Bug 1384256
- {
- stack: [
- "_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
- "openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
- "openPopup@chrome://global/content/bindings/autocomplete.xml",
- "set_popupOpen@chrome://global/content/bindings/autocomplete.xml",
- ],
- times: 3, // This number should only ever go down - never up.
- },
-
// Bug 1359989
{
stack: [
"openPopup@chrome://global/content/bindings/popup.xml",
"_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openPopup@chrome://global/content/bindings/autocomplete.xml",
"set_popupOpen@chrome://global/content/bindings/autocomplete.xml",
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -1872,59 +1872,73 @@ file, You can obtain one at http://mozil
<method name="_openAutocompletePopup">
<parameter name="aInput"/>
<parameter name="aElement"/>
<body><![CDATA[
if (this.mPopupOpen) {
return;
}
- this.mInput = aInput;
- aInput.controller.setInitiallySelectedIndex(this._isFirstResultHeuristic ? 0 : -1);
- this.view = aInput.controller.QueryInterface(Components.interfaces.nsITreeView);
- this._invalidate();
+ // Set the direction of the popup based on the textbox (bug 649840).
+ // getComputedStyle causes a layout flush, so avoid calling it if a
+ // direction has already been set.
+ if (!this.style.direction) {
+ this.style.direction =
+ aElement.ownerGlobal.getComputedStyle(aElement).direction;
+ }
+ let popupDirection = this.style.direction;
- var documentRect = window.document.documentElement.getBoundingClientRect();
- var width = documentRect.right - documentRect.left;
+ // Make the popup span the width of the window. First, set its width.
+ let documentRect =
+ this.DOMWindowUtils
+ .getBoundsWithoutFlushing(window.document.documentElement);
+ let width = documentRect.right - documentRect.left;
this.setAttribute("width", width);
- // Adjust the direction of the autocomplete popup list based on the textbox direction, bug 649840
- var popupDirection = aElement.ownerGlobal.getComputedStyle(aElement).direction;
- this.style.direction = popupDirection;
-
- // Make the popup's starting margin negative so that the leading edge
- // of the popup aligns with the window border.
- let elementRect = aElement.getBoundingClientRect();
+ // Now make its starting margin negative so that its leading edge
+ // aligns with the window border.
+ let elementRect =
+ this.DOMWindowUtils.getBoundsWithoutFlushing(aElement);
if (popupDirection == "rtl") {
let offset = elementRect.right - documentRect.right
this.style.marginRight = offset + "px";
} else {
let offset = documentRect.left - elementRect.left;
this.style.marginLeft = offset + "px";
}
// Keep the popup items' site icons aligned with the urlbar's identity
// icon if it's not too far from the edge of the window. We define
// "too far" as "more than 30% of the window's width AND more than
- // 250px"
+ // 250px". Do this *before* adding any items because when the new
+ // value of siteIconStart is different from the previous value, over-
+ // and underflow must be handled for each item already in the popup.
let boundToCheck = popupDirection == "rtl" ? "right" : "left";
let inputRect = this.DOMWindowUtils.getBoundsWithoutFlushing(aInput);
let startOffset = Math.abs(inputRect[boundToCheck] - documentRect[boundToCheck]);
let alignSiteIcons = startOffset / width <= 0.3 || startOffset <= 250;
if (alignSiteIcons) {
+ let identityIcon = document.getElementById("identity-icon");
let identityRect =
- document.getElementById("identity-icon").getBoundingClientRect();
+ this.DOMWindowUtils.getBoundsWithoutFlushing(identityIcon);
this.siteIconStart = popupDirection == "rtl" ? identityRect.right
: identityRect.left;
} else {
// Reset the alignment so that the site icons are positioned
// according to whatever's in the CSS.
this.siteIconStart = undefined;
}
+ // Now that siteIconStart has been set, start adding items (via
+ // _invalidate).
+ this.mInput = aInput;
+ aInput.controller.setInitiallySelectedIndex(this._isFirstResultHeuristic ? 0 : -1);
+ this.view = aInput.controller.QueryInterface(Components.interfaces.nsITreeView);
+ this._invalidate();
+
try {
let whichNotification = aInput.whichSearchSuggestionsNotification;
if (whichNotification != "none") {
// Update the impressions count on real popupshown, since there's
// no guarantee openPopup will be respected by the platform.
// Though, we must ensure the handled event is the expected one.
let impressionId = this._searchSuggestionsImpressionId = {};
this.addEventListener("popupshown", () => {
--- a/browser/extensions/formautofill/content/formautofill.xml
+++ b/browser/extensions/formautofill/content/formautofill.xml
@@ -40,21 +40,16 @@
this.removeAttribute("formautofillattached");
if (this._itemBox) {
this._itemBox.removeAttribute("size");
}
]]>
</body>
</method>
- <method name="_onChanged">
- <body>
- </body>
- </method>
-
<method name="_onOverflow">
<body></body>
</method>
<method name="_onUnderflow">
<body></body>
</method>
--- a/browser/themes/shared/urlbar-autocomplete.inc.css
+++ b/browser/themes/shared/urlbar-autocomplete.inc.css
@@ -11,16 +11,17 @@
.autocomplete-richlistbox {
padding: 4px 3px;
}
.autocomplete-richlistitem {
min-height: 30px;
font: message-box;
border-radius: 2px;
+ padding-inline-start: var(--item-padding-start);
}
:root[uidensity=touch] .autocomplete-richlistitem {
min-height: 40px;
}
.autocomplete-richlistitem:hover,
treechildren.searchbar-treebody::-moz-tree-row(hover) {
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1400,23 +1400,16 @@ extends="chrome://global/content/binding
item.collapsed = false;
} else {
// set the class at the end so we can use the attributes
// in the xbl constructor
item.className = "autocomplete-richlistitem";
this.richlistbox.appendChild(item);
}
- if (typeof item._onChanged == "function") {
- // The binding may have not been applied yet.
- setTimeout(() => {
- item._onChanged();
- }, 0);
- }
-
this._currentIndex++;
}
if (typeof this.onResultsAdded == "function")
this.onResultsAdded();
if (this._currentIndex < matchCount) {
// yield after each batch of items so that typing the url bar is
@@ -1430,21 +1423,30 @@ extends="chrome://global/content/binding
<!-- The x-coordinate relative to the leading edge of the window of the
items' site icons (favicons). -->
<property name="siteIconStart"
onget="return this._siteIconStart;">
<setter>
<![CDATA[
if (val != this._siteIconStart) {
this._siteIconStart = val;
+ let varName = "--item-padding-start";
+ if (typeof(val) == "number") {
+ let paddingInCSS =
+ 3 // .autocomplete-richlistbox padding-left/right
+ + 6 // .ac-site-icon margin-inline-start
+ + 16 // .ac-site-icon width
+ + 6; // .ac-site-icon margin-inline-end
+ let actualVal = Math.round(val) - paddingInCSS;
+ this.style.setProperty(varName, actualVal + "px");
+ } else {
+ this.style.removeProperty(varName);
+ }
for (let item of this.richlistbox.childNodes) {
- let changed = item.adjustSiteIconStart(val);
- if (changed) {
- item.handleOverUnderflow();
- }
+ item.handleOverUnderflow();
}
}
return val;
]]>
</setter>
</property>
<property name="overflowPadding"
@@ -1493,17 +1495,16 @@ extends="chrome://global/content/binding
</handler>
</handlers>
</binding>
<binding id="autocomplete-richlistitem-insecure-field" extends="chrome://global/content/bindings/autocomplete.xml#autocomplete-richlistitem">
<content align="center"
onoverflow="this._onOverflow();"
onunderflow="this._onUnderflow();">
- <xul:spacer anonid="type-icon-spacer"/>
<xul:image anonid="type-icon"
class="ac-type-icon"
xbl:inherits="selected,current,type"/>
<xul:image anonid="site-icon"
class="ac-site-icon"
xbl:inherits="src=image,selected,type"/>
<xul:vbox class="ac-title"
align="left"
@@ -1591,17 +1592,16 @@ extends="chrome://global/content/binding
</implementation>
</binding>
<binding id="autocomplete-richlistitem" extends="chrome://global/content/bindings/richlistbox.xml#richlistitem">
<content align="center"
onoverflow="this._onOverflow();"
onunderflow="this._onUnderflow();">
- <xul:spacer anonid="type-icon-spacer"/>
<xul:image anonid="type-icon"
class="ac-type-icon"
xbl:inherits="selected,current,type"/>
<xul:image anonid="site-icon"
class="ac-site-icon"
xbl:inherits="src=image,selected,type"/>
<xul:hbox class="ac-title"
align="center"
@@ -1646,19 +1646,16 @@ extends="chrome://global/content/binding
xbl:inherits="selected"/>
</xul:description>
</xul:hbox>
</content>
<implementation implements="nsIDOMXULSelectControlItemElement">
<constructor>
<![CDATA[
- this._typeIconSpacer = document.getAnonymousElementByAttribute(
- this, "anonid", "type-icon-spacer"
- );
this._typeIcon = document.getAnonymousElementByAttribute(
this, "anonid", "type-icon"
);
this._siteIcon = document.getAnonymousElementByAttribute(
this, "anonid", "site-icon"
);
this._titleText = document.getAnonymousElementByAttribute(
this, "anonid", "title-text"
@@ -2075,48 +2072,32 @@ extends="chrome://global/content/binding
Components.classes["@mozilla.org/intl/texttosuburi;1"]
.getService(Components.interfaces.nsITextToSubURI);
}
return this._textToSubURI.unEscapeURIForUI("UTF-8", url);
]]>
</body>
</method>
- <method name="_onChanged">
- <body>
- <![CDATA[
- let popup = this.parentNode.parentNode;
- let iconChanged = this.adjustSiteIconStart(popup._siteIconStart);
-
- if (iconChanged) {
- this.handleOverUnderflow();
- }
- ]]>
- </body>
- </method>
-
-
<method name="_reuseAcItem">
<body>
<![CDATA[
let action = this._parseActionUrl(this.getAttribute("url"));
let popup = this.parentNode.parentNode;
// If the item is a searchengine action, then it should
// only be reused if the engine name is the same as the
// popup's override engine name, if any.
if (!action ||
action.type != "searchengine" ||
!popup.overrideSearchEngineName ||
action.params.engineName == popup.overrideSearchEngineName) {
this.collapsed = false;
- // Call adjustSiteIconStart only after setting collapsed=
- // false. The calculations it does may be wrong otherwise.
- this.adjustSiteIconStart(popup._siteIconStart);
+
// The popup may have changed size between now and the last
// time the item was shown, so always handle over/underflow.
let dwu = window.getInterface(Ci.nsIDOMWindowUtils);
let popupWidth = dwu.getBoundsWithoutFlushing(this.parentNode).width;
if (!this._previousPopupWidth || this._previousPopupWidth != popupWidth) {
this._previousPopupWidth = popupWidth;
this.handleOverUnderflow();
}
@@ -2374,50 +2355,16 @@ extends="chrome://global/content/binding
this._urlText.style.removeProperty("max-width");
this._actionText.style.removeProperty("max-width");
this._hasMaxWidths = false;
}
]]>
</body>
</method>
- <!-- Sets the x-coordinate of the leading edge of the site icon (favicon)
- relative the the leading edge of the window.
- @param newStart The new x-coordinate, relative to the leading edge of
- the window. Pass undefined to reset the icon's position to
- whatever is specified in CSS.
- @return True if the icon's position changed, false if not. -->
- <method name="adjustSiteIconStart">
- <parameter name="newStart"/>
- <body>
- <![CDATA[
- if (typeof(newStart) != "number") {
- this._typeIconSpacer.style.removeProperty("width");
- return true;
- }
-
- let utils = window.QueryInterface(Ci.nsIInterfaceRequestor)
- .getInterface(Ci.nsIDOMWindowUtils);
- let rect = utils.getBoundsWithoutFlushing(this._siteIcon);
-
- let dir = this.getAttribute("dir");
- let delta = dir == "rtl" ? rect.right - Math.round(newStart)
- : Math.round(newStart) - rect.left;
- if (delta) {
- let currentSpacerWidth = this._typeIconSpacer.style.width || "0px";
- this._typeIconSpacer.style.width =
- (parseInt(currentSpacerWidth, 10) + delta) + "px";
- return true;
- }
-
- return false;
- ]]>
- </body>
- </method>
-
<!-- This method truncates the displayed strings as necessary. -->
<method name="_handleOverflow">
<body><![CDATA[
let itemRect = this.parentNode.getBoundingClientRect();
let titleRect = this._titleText.getBoundingClientRect();
let tagsRect = this._tagsText.getBoundingClientRect();
let separatorRect = this._separator.getBoundingClientRect();
let urlRect = this._urlText.getBoundingClientRect();
--- a/toolkit/themes/linux/global/autocomplete.css
+++ b/toolkit/themes/linux/global/autocomplete.css
@@ -109,17 +109,17 @@ treechildren.autocomplete-treebody::-moz
color: HighlightText;
}
.ac-type-icon {
width: 16px;
height: 16px;
max-width: 16px;
max-height: 16px;
- margin-inline-start: 13px;
+ margin-inline-start: 6px;
margin-inline-end: 6px;
}
.ac-site-icon {
width: 16px;
height: 16px;
max-width: 16px;
max-height: 16px;
--- a/toolkit/themes/osx/global/autocomplete.css
+++ b/toolkit/themes/osx/global/autocomplete.css
@@ -93,17 +93,17 @@ treechildren.autocomplete-treebody::-moz
margin: 0;
}
.ac-type-icon {
width: 16px;
height: 16px;
max-width: 16px;
max-height: 16px;
- margin-inline-start: 16px;
+ margin-inline-start: 6px;
margin-inline-end: 6px;
}
.ac-site-icon {
width: 16px;
height: 16px;
max-width: 16px;
max-height: 16px;
--- a/toolkit/themes/windows/global/autocomplete.css
+++ b/toolkit/themes/windows/global/autocomplete.css
@@ -102,17 +102,17 @@ treechildren.autocomplete-treebody::-moz
color: HighlightText;
}
.ac-type-icon {
width: 16px;
height: 16px;
max-width: 16px;
max-height: 16px;
- margin-inline-start: 14px;
+ margin-inline-start: 6px;
margin-inline-end: 6px;
}
.ac-site-icon {
width: 16px;
height: 16px;
max-width: 16px;
max-height: 16px;