Bug 1392081 - Set the image attribute on bookmarks toolbar buttons only when they are visible. r=Gijs
authorMarco Bonardo <mbonardo@mozilla.com>
Fri, 25 Aug 2017 15:31:06 +0200
changeset 428927 3f2d6fc1693d5890f6d15d3b5270e0853875e757
parent 428926 8cabc32862b8b7a432957399dda633d9bff3934e
child 428928 b3a7b5c0618ba270ed97374ee71eb39ba7a98a3a
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1392081
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1392081 - Set the image attribute on bookmarks toolbar buttons only when they are visible. r=Gijs MozReview-Commit-ID: HIalcxcCBsv
browser/components/places/content/browserPlacesViews.js
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -1058,19 +1058,16 @@ PlacesToolbar.prototype = {
     let type = aChild.type;
     let button;
     if (type == Ci.nsINavHistoryResultNode.RESULT_TYPE_SEPARATOR) {
       button = document.createElement("toolbarseparator");
     } else {
       button = document.createElement("toolbarbutton");
       button.className = "bookmark-item";
       button.setAttribute("label", aChild.title || "");
-      let icon = aChild.icon;
-      if (icon)
-        button.setAttribute("image", icon);
 
       if (PlacesUtils.containerTypes.includes(type)) {
         button.setAttribute("type", "menu");
         button.setAttribute("container", "true");
 
         if (PlacesUtils.nodeIsQuery(aChild)) {
           button.setAttribute("query", "true");
           if (PlacesUtils.nodeIsTagQuery(aChild))
@@ -1236,66 +1233,83 @@ PlacesToolbar.prototype = {
       let child = this._rootElt.childNodes[i];
       // Once a child overflows, all the next ones will.
       if (!childOverflowed) {
         let childRect = child.getBoundingClientRect();
         childOverflowed = this.isRTL ? (childRect.left < scrollRect.left)
                                      : (childRect.right > scrollRect.right);
 
       }
-      child.style.visibility = childOverflowed ? "hidden" : "visible";
+      if (childOverflowed) {
+        child.removeAttribute("image");
+        child.style.visibility = "hidden";
+      } else {
+        let icon = child._placesNode.icon;
+        if (icon)
+          child.setAttribute("image", icon);
+        child.style.visibility = "visible";
+      }
+
     }
 
     // We rebuild the chevron on popupShowing, so if it is open
     // we must update it.
     if (this._chevron.open)
       this._updateChevronPopupNodesVisibility();
   },
 
   nodeInserted:
   function PT_nodeInserted(aParentPlacesNode, aPlacesNode, aIndex) {
     let parentElt = this._getDOMNodeForPlacesNode(aParentPlacesNode);
-    if (parentElt == this._rootElt) {
+    if (parentElt == this._rootElt) { // Node is on the toolbar.
       let children = this._rootElt.childNodes;
-      this._insertNewItem(aPlacesNode, this._rootElt,
-        aIndex < children.length ? children[aIndex] : null);
-      this.updateChevron();
+      let button = this._insertNewItem(aPlacesNode, this._rootElt,
+                                       children[aIndex] || null);
+      let prevSiblingOverflowed = aIndex > 0 && aIndex <= children.length &&
+                                  children[aIndex - 1].style.visibility == "hidden";
+      if (prevSiblingOverflowed) {
+        button.style.visibility = "hidden";
+      } else {
+        let icon = aPlacesNode.icon;
+        if (icon)
+          button.setAttribute("image", icon);
+        this.updateChevron();
+      }
       return;
     }
 
     PlacesViewBase.prototype.nodeInserted.apply(this, arguments);
   },
 
   nodeRemoved:
   function PT_nodeRemoved(aParentPlacesNode, aPlacesNode, aIndex) {
     let parentElt = this._getDOMNodeForPlacesNode(aParentPlacesNode);
     let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
 
     // Here we need the <menu>.
     if (elt.localName == "menupopup")
       elt = elt.parentNode;
 
-    if (parentElt == this._rootElt) {
+    if (parentElt == this._rootElt) { // Node is on the toolbar.
+      let overflowed = elt.style.visibility == "hidden";
       this._removeChild(elt);
-      this.updateChevron();
+      if (!overflowed)
+        this.updateChevron();
       return;
     }
 
     PlacesViewBase.prototype.nodeRemoved.apply(this, arguments);
   },
 
   nodeMoved:
   function PT_nodeMoved(aPlacesNode,
                         aOldParentPlacesNode, aOldIndex,
                         aNewParentPlacesNode, aNewIndex) {
     let parentElt = this._getDOMNodeForPlacesNode(aNewParentPlacesNode);
-    if (parentElt == this._rootElt) {
-      // Container is on the toolbar.
-
-      // Move the element.
+    if (parentElt == this._rootElt) { // Node is on the toolbar.
       let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
 
       // Here we need the <menu>.
       if (elt.localName == "menupopup")
         elt = elt.parentNode;
 
       this._removeChild(elt);
       this._rootElt.insertBefore(elt, this._rootElt.childNodes[aNewIndex]);
@@ -1319,19 +1333,17 @@ PlacesToolbar.prototype = {
     let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
     if (elt == this._rootElt)
       return;
 
     // We're notified for the menupopup, not the containing toolbarbutton.
     if (elt.localName == "menupopup")
       elt = elt.parentNode;
 
-    if (elt.parentNode == this._rootElt) {
-      // Node is on the toolbar.
-
+    if (elt.parentNode == this._rootElt) { // Node is on the toolbar.
       // All livemarks have a feedURI, so use it as our indicator.
       if (aAnno == PlacesUtils.LMANNO_FEEDURI) {
         elt.setAttribute("livemark", true);
 
         PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId })
           .then(aLivemark => {
             this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark);
             this.invalidateContainer(aPlacesNode);
@@ -1352,19 +1364,19 @@ PlacesToolbar.prototype = {
       return;
 
     PlacesViewBase.prototype.nodeTitleChanged.apply(this, arguments);
 
     // Here we need the <menu>.
     if (elt.localName == "menupopup")
       elt = elt.parentNode;
 
-    if (elt.parentNode == this._rootElt) {
-      // Node is on the toolbar
-      this.updateChevron();
+    if (elt.parentNode == this._rootElt) { // Node is on the toolbar.
+      if (elt.style.visibility != "hidden")
+        this.updateChevron();
     }
   },
 
   invalidateContainer: function PT_invalidateContainer(aPlacesNode) {
     let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
     if (elt == this._rootElt) {
       // Container is the toolbar itself.
       this._rebuild();
@@ -1511,17 +1523,19 @@ PlacesToolbar.prototype = {
     let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
     timer.initWithCallback(this, aTime, timer.TYPE_ONE_SHOT);
     return timer;
   },
 
   notify: function PT_notify(aTimer) {
     if (aTimer == this._updateChevronTimer) {
       this._updateChevronTimer = null;
-      this._updateChevronTimerCallback();
+      BrowserUtils.promiseLayoutFlushed(document, "layout", () => {
+        window.requestAnimationFrame(this._updateChevronTimerCallback);
+      });
     } else if (aTimer == this._ibTimer) {
       // * Timer to turn off indicator bar.
       this._dropIndicator.collapsed = true;
       this._ibTimer = null;
     } else if (aTimer == this._overFolder.openTimer) {
       // * Timer to open a menubutton that's being dragged over.
       // Set the autoopen attribute on the folder's menupopup so that
       // the menu will automatically close when the mouse drags off of it.
@@ -1917,16 +1931,17 @@ PlacesPanelMenuView.prototype = {
       }
     }
 
     button._placesNode = aChild;
     if (!this._domNodes.has(aChild))
       this._domNodes.set(aChild, button);
 
     aInsertionNode.insertBefore(button, aBefore);
+    return button;
   },
 
   nodeInserted:
   function PAMV_nodeInserted(aParentPlacesNode, aPlacesNode, aIndex) {
     let parentElt = this._getDOMNodeForPlacesNode(aParentPlacesNode);
     if (parentElt != this._rootElt)
       return;