Bug 1381427 - Result width should often be constrained by the width of the location bar. r=mak, a=ritu
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Sat, 21 Oct 2017 15:17:08 +0100
changeset 432688 22ad3b8731eb021bb3c93f5992265ea88b4d7c31
parent 432687 4d583d584b5c3a30dd7146eae9c5b218d5eb83c6
child 432689 938e8d1ced3df5ec4a033e8fbcaa24efef9d42e2
push id8029
push userryanvm@gmail.com
push dateMon, 23 Oct 2017 18:08:40 +0000
treeherdermozilla-beta@86534d5daeef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, ritu
bugs1381427
milestone57.0
Bug 1381427 - Result width should often be constrained by the width of the location bar. r=mak, a=ritu This adds an end margin to the location bar search results that matches the end of the input field, unless doing this would be unbalanced with the start margin, in which case a symmetrical margin is chosen. This allows the location bar to be moved to the start of the navigation toolbar to reclaim space for results. This also handles gracefully other customization scenarios like having many icons on the right side of the location bar. MozReview-Commit-ID: CjvbTtn7VVh
browser/base/content/browser.xul
browser/base/content/urlbarBindings.xml
browser/themes/shared/urlbar-autocomplete.inc.css
toolkit/content/widgets/autocomplete.xml
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -153,17 +153,17 @@
 
     <!-- for url bar autocomplete -->
     <panel type="autocomplete-richlistbox"
            id="PopupAutoCompleteRichResult"
            noautofocus="true"
            hidden="true"
            flip="none"
            level="parent"
-           overflowpadding="30" />
+           overflowpadding="15" />
 
    <!-- for date/time picker. consumeoutsideclicks is set to never, so that
         clicks on the anchored input box are never consumed. -->
     <panel id="DateTimePickerPanel"
            type="arrow"
            hidden="true"
            orient="vertical"
            noautofocus="true"
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -1853,16 +1853,56 @@ file, You can obtain one at http://mozil
         </getter>
         <setter>
           <![CDATA[
             return this._maxResults = parseInt(val);
           ]]>
         </setter>
       </property>
 
+      <!-- This is set either to undefined or to a new object containing
+           { start, end } margin values in pixels. These are used to align the
+           results to the input field. -->
+      <property name="margins"
+                onget="return this._margins;">
+        <setter>
+          <![CDATA[
+          if (val == this._margins) {
+            return val;
+          }
+
+          if (val && this._margins && val.start == this._margins.start &&
+                                      val.end == this._margins.end) {
+            return val;
+          }
+
+          this._margins = val;
+
+          if (val) {
+            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.start) - paddingInCSS;
+            let actualValEnd = Math.round(val.end);
+            this.style.setProperty("--item-padding-start", actualVal + "px");
+            this.style.setProperty("--item-padding-end", actualValEnd + "px");
+          } else {
+            this.style.removeProperty("--item-padding-start");
+            this.style.removeProperty("--item-padding-end");
+          }
+          for (let item of this.richlistbox.childNodes) {
+            item.handleOverUnderflow();
+          }
+          return val;
+          ]]>
+        </setter>
+      </property>
+
       <method name="openAutocompletePopup">
         <parameter name="aInput"/>
         <parameter name="aElement"/>
         <body>
           <![CDATA[
           // initially the panel is hidden
           // to avoid impacting startup / new window performance
           aInput.popup.hidden = false;
@@ -1907,38 +1947,50 @@ file, You can obtain one at http://mozil
             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".  Do this *before* adding any items because when the new
-          // value of siteIconStart is different from the previous value, over-
+          // value of the margins are 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) {
+            // Calculate the end margin if we have a start margin.
+            let boundToCheckEnd = popupDirection == "rtl" ? "left" : "right";
+            let endOffset = Math.abs(inputRect[boundToCheckEnd] -
+                                     documentRect[boundToCheckEnd]);
+            if (endOffset > startOffset * 2) {
+              // Provide more space when aligning would result in an unbalanced
+              // margin. This allows the location bar to be moved to the start
+              // of the navigation toolbar to reclaim space for results.
+              endOffset = startOffset;
+            }
             let identityIcon = document.getElementById("identity-icon");
             let identityRect =
               this.DOMWindowUtils.getBoundsWithoutFlushing(identityIcon);
             if (popupDirection == "rtl") {
-              this.siteIconStart = documentRect.right - identityRect.right;
+              this.margins = { start: documentRect.right - identityRect.right,
+                               end: endOffset };
             } else {
-              this.siteIconStart = identityRect.left;
+              this.margins = { start: identityRect.left,
+                               end: endOffset };
             }
           } else {
             // Reset the alignment so that the site icons are positioned
             // according to whatever's in the CSS.
-            this.siteIconStart = undefined;
+            this.margins = undefined;
           }
 
-          // Now that siteIconStart has been set, start adding items (via
+          // Now that the margins have 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;
@@ -1975,19 +2027,19 @@ file, You can obtain one at http://mozil
         <parameter name="whichNotification"/>
         <parameter name="popupDirection"/>
         <body>
           <![CDATA[
           let deckIndex = 0;
           if (whichNotification == "opt-out") {
             deckIndex = 1;
 
-            if (this.siteIconStart) {
+            if (this.margins) {
               this.searchSuggestionsNotification.style.paddingInlineStart =
-                this.siteIconStart + "px";
+                this.margins.start + "px";
             } else {
               this.searchSuggestionsNotification.style.removeProperty("padding-inline-start");
             }
 
             // We want to animate the opt-out hint only once.
             if (!this._firstSearchSuggestionsNotification) {
               this._firstSearchSuggestionsNotification = true;
               this.searchSuggestionsNotification.setAttribute("animate", "true");
--- a/browser/themes/shared/urlbar-autocomplete.inc.css
+++ b/browser/themes/shared/urlbar-autocomplete.inc.css
@@ -12,16 +12,21 @@
   padding: 4px 3px;
 }
 
 .autocomplete-richlistitem {
   min-height: 30px;
   font: message-box;
   border-radius: 2px;
   padding-inline-start: var(--item-padding-start);
+  /* For the space after the autocomplete text we have to use a transparent
+     border instead of padding, because the latter would considered part of the
+     scrollable area when generating the overflow events that we use to
+     constrain the autocomplete result item size. */
+  border-inline-end: var(--item-padding-end) solid transparent;
 }
 
 :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
@@ -1415,45 +1415,16 @@ extends="chrome://global/content/binding
             // yield after each batch of items so that typing the url bar is
             // responsive
             this._appendResultTimeout = setTimeout(() => this._appendCurrentResult(), 0);
           }
         ]]>
         </body>
       </method>
 
-      <!-- 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) {
-              item.handleOverUnderflow();
-            }
-          }
-          return val;
-          ]]>
-        </setter>
-      </property>
-
       <property name="overflowPadding"
                 onget="return Number(this.getAttribute('overflowpadding'))"
                 readonly="true" />
 
       <method name="selectBy">
         <parameter name="aReverse"/>
         <parameter name="aPage"/>
         <body>
@@ -2376,17 +2347,18 @@ extends="chrome://global/content/binding
           // minus the start of the title text minus a little optional extra padding.
           // This extra padding amount is basically arbitrary but keeps the text
           // from getting too close to the popup's edge.
           let dir = this.getAttribute("dir");
           let titleStart = dir == "rtl" ? itemRect.right - titleRect.right
                                         : titleRect.left - itemRect.left;
 
           let popup = this.parentNode.parentNode;
-          let itemWidth = itemRect.width - titleStart - popup.overflowPadding;
+          let itemWidth = itemRect.width - titleStart - popup.overflowPadding -
+                          (popup.margins ? popup.margins.end : 0);
 
           if (this._tags.hasAttribute("empty")) {
             // The tags box is not displayed in this case.
             tagsRect.width = 0;
           }
 
           let titleTagsWidth = titleRect.width + tagsRect.width;
           if (titleTagsWidth + separatorURLActionWidth > itemWidth) {