Bug 1262588 - Keep favicons in awesomebar popup aligned with urlbar's identity icon. r?mak draft
authorDrew Willcoxon <adw@mozilla.com>
Tue, 12 Apr 2016 19:59:22 -0700
changeset 350254 43db2e04bfd8ec08b45c3142adf8ccd4993d0583
parent 349990 c285601ae929fd934182191288fe8d733ff0aee3
child 518280 dceecfe482d51114bcd8b63a802215dd206f4861
push id15284
push userdwillcoxon@mozilla.com
push dateWed, 13 Apr 2016 02:59:32 +0000
reviewersmak
bugs1262588
milestone48.0a1
Bug 1262588 - Keep favicons in awesomebar popup aligned with urlbar's identity icon. r?mak MozReview-Commit-ID: ADixnHLNA5y
browser/base/content/urlbarBindings.xml
toolkit/content/widgets/autocomplete.xml
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -1355,27 +1355,44 @@ file, You can obtain one at http://mozil
           var rect = window.document.documentElement.getBoundingClientRect();
           var width = rect.right - rect.left;
           this.setAttribute("width", width);
 
           // Adjust the direction of the autocomplete popup list based on the textbox direction, bug 649840
           var popupDirection = aElement.ownerDocument.defaultView.getComputedStyle(aElement).direction;
           this.style.direction = popupDirection;
 
-          // Make the popup's starting margin negaxtive so that the start of the
-          // popup aligns with the window border.
+          // Make the popup's starting margin negative so that the leading edge
+          // of the popup aligns with the window border.
           let elementRect = aElement.getBoundingClientRect();
           if (popupDirection == "rtl") {
             let offset = elementRect.right - rect.right
             this.style.marginRight = offset + "px";
           } else {
             let offset = rect.left - elementRect.left;
             this.style.marginLeft = offset + "px";
           }
 
+          // Keep the popup items' site icons aligned with the identity icon in
+          // the urlbar, but only if it's not too far from the edge of the
+          // window.  The forward button's visibility may have changed since the
+          // last time the popup was opened, so that needs to happen now.  Do it
+          // *before* the popup opens because otherwise the items will visibly
+          // shift.
+          let identityRect =
+            document.getElementById("identity-icon").getBoundingClientRect();
+          let siteIconStart = popupDirection == "rtl" ? identityRect.right
+                                                      : identityRect.left;
+          // 100 is basically arbitrary, but it's more than enough to cover the
+          // case when the forward button is shown and there's nothing before
+          // the back-forward buttons.
+          if (siteIconStart < 100) {
+            this.siteIconStart = siteIconStart;
+          }
+
           // Position the popup below the navbar.  To get the y-coordinate,
           // which is an offset from the bottom of the input, subtract the
           // bottom of the navbar from the buttom of the input.
           let yOffset =
             document.getElementById("nav-bar").getBoundingClientRect().bottom -
             aInput.getBoundingClientRect().bottom;
           this.openPopup(aElement, "after_start", 0, yOffset, false, false);
         ]]></body>
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1219,16 +1219,19 @@ extends="chrome://global/content/binding
       <method name="_appendCurrentResult">
         <parameter name="invalidateReason"/>
         <body>
           <![CDATA[
           var controller = this.mInput.controller;
           var matchCount = this._matchCount;
           var existingItemsCount = this.richlistbox.childNodes.length;
 
+          let popupDir =
+            this.ownerDocument.defaultView.getComputedStyle(this).direction;
+
           // Process maxRows per chunk to improve performance and user experience
           for (let i = 0; i < this.maxRows; i++) {
             if (this._currentIndex >= matchCount)
               break;
 
             var item;
 
             // trim the leading/trailing whitespace
@@ -1244,17 +1247,20 @@ extends="chrome://global/content/binding
               // due to new results, but only when: the item is the same, *OR*
               // we are about to replace the currently mouse-selected item, to
               // avoid surprising the user.
               let iface = Components.interfaces.nsIAutoCompletePopup;
               if (item.getAttribute("text") == trimmedSearchString &&
                   invalidateReason == iface.INVALIDATE_REASON_NEW_RESULT &&
                   (item.getAttribute("url") == url ||
                    this.richlistbox.mouseSelectedIndex === this._currentIndex)) {
-                item.handleOverUnderflow();
+                // The popup may have changed size between now and the last time
+                // the item was shown, so pass true as the third argument to
+                // force over/underflow to be handled.
+                item.setSiteIconStart(this._siteIconStart, popupDir, true);
                 item.collapsed = false;
                 this._currentIndex++;
                 continue;
               }
             }
             else {
               // need to create a new item
               item = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "richlistitem");
@@ -1276,31 +1282,52 @@ extends="chrome://global/content/binding
             }
             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);
             }
 
+            item.setSiteIconStart(this._siteIconStart, popupDir, false);
+
             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
             // 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 popupDir =
+              this.ownerDocument.defaultView.getComputedStyle(this).direction;
+            for (let item of this.richlistbox.childNodes) {
+              item.setSiteIconStart(val, popupDir, false);
+            }
+          }
+          return val;
+          ]]>
+        </setter>
+      </property>
+
       <method name="selectBy">
         <parameter name="aReverse"/>
         <parameter name="aPage"/>
         <body>
           <![CDATA[
           try {
             var amount = aPage ? 5 : 1;
 
@@ -1992,16 +2019,54 @@ extends="chrome://global/content/binding
           this._titleText.style.removeProperty("max-width");
           this._tagsText.style.removeProperty("max-width");
           this._urlText.style.removeProperty("max-width");
           this._actionText.style.removeProperty("max-width");
           ]]>
         </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.  May be undefined if you just
+                  want to force handling over/underflow.
+           @param popupDir The popup direction, "ltr" or "rtl".
+           @param forceHandleOverflow Normally over/underflow is handled only if
+                  the new start is different from the old start.  Pass true for
+                  this parameter to handle it regardless. -->
+      <method name="setSiteIconStart">
+        <parameter name="newStart"/>
+        <parameter name="popupDir"/>
+        <parameter name="forceHandleOverflow"/>
+        <body>
+          <![CDATA[
+          let delta = 0;
+          // Allow callers to pass in newStart=undefined but
+          // forceHandleOverflow=true to force handling over/underflow.
+          if (typeof(newStart) == "number") {
+            let rect = this._siteIcon.getBoundingClientRect();
+            let oldStart = popupDir == "rtl" ? rect.right : rect.left;
+            let delta = newStart - oldStart;
+            let px = this._typeIcon.style.MozMarginStart;
+            if (!px) {
+              // Allow -moz-margin-start not to be specified in CSS initially.
+              let style = window.getComputedStyle(this._typeIcon);
+              px = popupDir == "rtl" ? style.marginRight : style.marginLeft;
+            }
+            let typeIconStart = Number(px.substr(0, px.length - 2));
+            this._typeIcon.style.MozMarginStart =
+              (typeIconStart + delta) + "px";
+          }
+          if (delta != 0 || forceHandleOverflow) {
+            this._handleOverUnderflow();
+          }
+          ]]>
+        </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();
@@ -2063,17 +2128,17 @@ extends="chrome://global/content/binding
             );
             urlActionMaxWidth -= separatorRect.width;
             this._urlText.style.maxWidth = urlActionMaxWidth + "px";
             this._actionText.style.maxWidth = urlActionMaxWidth + "px";
           }
         ]]></body>
       </method>
 
-      <method name="handleOverUnderflow">
+      <method name="_handleOverUnderflow">
         <body>
           <![CDATA[
           this._removeMaxWidths();
           this._handleOverflow();
           ]]>
         </body>
       </method>