Bug 1262588 - Keep favicons in awesomebar popup aligned with urlbar's identity icon. r=mak draft
authorDrew Willcoxon <adw@mozilla.com>
Fri, 15 Apr 2016 11:19:07 -0700
changeset 352099 4414044a762529b9cd5d7cb6b4dd9e767ff40188
parent 352098 52e570214af308e604b413645136f340ad412711
child 518578 7916b7883e7c85e2b52a9465baf14b2e1ddc55fd
push id15615
push userdwillcoxon@mozilla.com
push dateFri, 15 Apr 2016 18:20:24 +0000
reviewersmak
bugs1262588
milestone48.0a1
Bug 1262588 - Keep favicons in awesomebar popup aligned with urlbar's identity icon. r=mak MozReview-Commit-ID: DEqmPgCHK3U
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,51 @@ 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 urlbar's identity
+          // icon if it's not too far from the edge of the window.  If there are
+          // at most two toolbar buttons between the window edge and the urlbar,
+          // then consider that as "not too far."  The forward button's
+          // visibility may have changed since the last time the popup was
+          // opened, so this needs to happen now.  Do it *before* the popup
+          // opens because otherwise the items will visibly shift.
+          let nodes = [...document.getElementById("nav-bar-customization-target").childNodes];
+          let urlbarPosition = nodes.findIndex(n => n.id == "urlbar-container");
+          let alignSiteIcons = urlbarPosition <= 2 &&
+                               nodes.slice(0, urlbarPosition)
+                                    .every(n => n.localName == "toolbarbutton");
+          if (alignSiteIcons) {
+            let identityRect =
+              document.getElementById("identity-icon").getBoundingClientRect();
+            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;
+          }
+
           // 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
@@ -1234,35 +1234,42 @@ extends="chrome://global/content/binding
             // trim the leading/trailing whitespace
             var trimmedSearchString = controller.searchString.replace(/^\s+/, "").replace(/\s+$/, "");
 
             let url = controller.getValueAt(this._currentIndex);
 
             if (this._currentIndex < existingItemsCount) {
               // re-use the existing item
               item = this.richlistbox.childNodes[this._currentIndex];
+              item.setAttribute("dir", this.style.direction);
 
               // Completely reuse the existing richlistitem for invalidation
               // 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.collapsed = false;
+                // Call adjustSiteIconStart only after setting collapsed=false.
+                // The calculations it does may be wrong otherwise.
+                item.adjustSiteIconStart(this._siteIconStart);
+                // The popup may have changed size between now and the last time
+                // the item was shown, so always handle over/underflow.
                 item.handleOverUnderflow();
-                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");
+              item.setAttribute("dir", this.style.direction);
             }
 
             // set these attributes before we set the class
             // so that we can use them from the constructor
             let iconURI = controller.getImageAt(this._currentIndex);
             item.setAttribute("image", iconURI);
             item.setAttribute("url", url);
             item.setAttribute("title", controller.getCommentAt(this._currentIndex));
@@ -1276,31 +1283,56 @@ 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);
             }
 
+            let changed = item.adjustSiteIconStart(this._siteIconStart);
+            if (changed) {
+              item.handleOverUnderflow();
+            }
+
             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;
+            for (let item of this.richlistbox.childNodes) {
+              let changed = item.adjustSiteIconStart(val);
+              if (changed) {
+                item.handleOverUnderflow();
+              }
+            }
+          }
+          return val;
+          ]]>
+        </setter>
+      </property>
+
       <method name="selectBy">
         <parameter name="aReverse"/>
         <parameter name="aPage"/>
         <body>
           <![CDATA[
           try {
             var amount = aPage ? 5 : 1;
 
@@ -1992,16 +2024,47 @@ 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, 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._typeIcon.style.removeProperty("-moz-margin-start");
+            return true;
+          }
+          let rect = this._siteIcon.getBoundingClientRect();
+          let dir = this.getAttribute("dir");
+          let delta = dir == "rtl" ? rect.right - newStart
+                                   : newStart - rect.left;
+          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 = dir == "rtl" ? style.marginRight : style.marginLeft;
+          }
+          let typeIconStart = Number(px.substr(0, px.length - 2));
+          this._typeIcon.style.MozMarginStart = (typeIconStart + delta) + "px";
+          return delta > 0;
+          ]]>
+        </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();
@@ -2009,18 +2072,17 @@ extends="chrome://global/content/binding
           let separatorURLActionWidth =
             separatorRect.width + Math.max(urlRect.width, actionRect.width);
 
           // Total width for the title and URL/action is the width of the item
           // minus the start of the title text minus a little extra padding.
           // This extra padding amount is basically arbitrary but keeps the text
           // from getting too close to the popup's edge.
           let extraPadding = 30;
-          let dir =
-            this.ownerDocument.defaultView.getComputedStyle(this).direction;
+          let dir = this.getAttribute("dir");
           let titleStart = dir == "rtl" ? itemRect.right - titleRect.right
                                         : titleRect.left - itemRect.left;
           let itemWidth = itemRect.width - titleStart - extraPadding;
 
           if (this._tags.hasAttribute("empty")) {
             // The tags box is not displayed in this case.
             tagsRect.width = 0;
           }