Bug 1381427 - Result width should often be constrained by the width of the location bar. r=mak
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Sat, 21 Oct 2017 15:17:08 +0100
changeset 684380 20d45a0a9a7bc8ef8159823e07fd0b2380bfb39a
parent 684379 30938a11b6e89382869a626f7ce0cd92de4028a3
child 684459 125a67a1750fab30926dfeff6b5d838092317dbc
push id85600
push userpaolo.mozmail@amadzone.org
push dateSat, 21 Oct 2017 14:20:47 +0000
reviewersmak
bugs1381427
milestone58.0a1
Bug 1381427 - Result width should often be constrained by the width of the location bar. r=mak 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: 9TwieL0lqHB
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
@@ -1803,16 +1803,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;
@@ -1857,38 +1897,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;
@@ -1922,19 +1974,19 @@ file, You can obtain one at http://mozil
       </method>
 
       <method name="_showSearchSuggestionsNotification">
         <parameter name="whichNotification"/>
         <parameter name="popupDirection"/>
         <body>
           <![CDATA[
           if (whichNotification == "opt-out") {
-            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
@@ -1377,45 +1377,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>
@@ -2336,17 +2307,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) {