Bug 1326138 - Refactor _appendCurrentResult to be more generic. r=adw
authorRay Lin <ralin@mozilla.com>
Tue, 24 Jan 2017 23:32:36 +0800
changeset 343261 b25738421d6e3390548b27c224cdcd2dfa613133
parent 343260 dce3a8ef420554b81f5541c9248ee4f50de42bcc
child 343262 bd3f65389f27a9effc74b36dc7c03672e84a661d
push id31374
push userkwierso@gmail.com
push dateThu, 16 Feb 2017 17:26:30 +0000
treeherdermozilla-central@4158b1d8bb2a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1326138
milestone54.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 1326138 - Refactor _appendCurrentResult to be more generic. r=adw MozReview-Commit-ID: LTDVtiOYbx6
toolkit/content/widgets/autocomplete.xml
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1299,92 +1299,92 @@ extends="chrome://global/content/binding
         <body>
           <![CDATA[
           var controller = this.mInput.controller;
           var matchCount = this._matchCount;
           var existingItemsCount = this.richlistbox.childNodes.length;
 
           // Process maxRows per chunk to improve performance and user experience
           for (let i = 0; i < this.maxRows; i++) {
-            if (this._currentIndex >= matchCount)
+            if (this._currentIndex >= matchCount) {
               break;
-
-            var item;
-
-            // 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);
+            }
+            let item;
+            let reusable = false;
+            let itemExists = this._currentIndex < existingItemsCount;
 
-              // 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)) {
-                // Additionally, if the item is a searchengine action, then it
-                // should only be reused if the engine name is the same as the
-                // popup's override engine name, if any.
-                let action = item._parseActionUrl(url);
-                if (!action ||
-                    action.type != "searchengine" ||
-                    !this.overrideSearchEngineName ||
-                    action.params.engineName == this.overrideSearchEngineName) {
-                  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();
-                  this._currentIndex++;
-                  continue;
-                }
-              }
+            let originalValue, originalText;
+            let value = controller.getValueAt(this._currentIndex);
+            let label = controller.getLabelAt(this._currentIndex);
+            let comment = controller.getCommentAt(this._currentIndex);
+            let style = controller.getStyleAt(this._currentIndex);
+            let image = controller.getImageAt(this._currentIndex);
+            // trim the leading/trailing whitespace
+            let trimmedSearchString = controller.searchString.replace(/^\s+/, "").replace(/\s+$/, "");
+
+            if (itemExists) {
+              item = this.richlistbox.childNodes[this._currentIndex];
+
+              originalValue = item.getAttribute("ac-value");
+              originalText = item.getAttribute("ac-text");
+
+              reusable = item.getAttribute("originaltype") === style;
             } 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));
-            item.setAttribute("originaltype", controller.getStyleAt(this._currentIndex));
-            item.setAttribute("text", trimmedSearchString);
+            item.setAttribute("dir", this.style.direction);
+            item.setAttribute("ac-image", image);
+            item.setAttribute("ac-value", value);
+            item.setAttribute("ac-label", label);
+            item.setAttribute("ac-comment", comment);
+            item.setAttribute("ac-text", trimmedSearchString);
 
-            if (this._currentIndex < existingItemsCount) {
-              // re-use the existing item
+            // 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 (reusable &&
+                originalText == trimmedSearchString &&
+                invalidateReason == iface.INVALIDATE_REASON_NEW_RESULT &&
+                (originalValue == value ||
+                 this.richlistbox.mouseSelectedIndex === this._currentIndex)) {
+
+              // try to re-use the existing item
+              let reused = item._reuseAcItem();
+              if (reused) {
+                this._currentIndex++;
+                continue;
+              }
+            } else {
+              if (typeof item._cleanup == "function") {
+                item._cleanup();
+              }
+
+              item.setAttribute("originaltype", style);
+            }
+
+            if (itemExists) {
               item._adjustAcItem();
               item.collapsed = false;
             } 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);
             }
 
-            // The binding may have not been applied yet.
-            setTimeout(() => {
-              let changed = item.adjustSiteIconStart(this._siteIconStart);
-              if (changed) {
-                item.handleOverUnderflow();
-              }
-            }, 0);
+            if (typeof item.onChanged == "function") {
+              // The binding may have not been applied yet.
+              setTimeout(() => {
+                item.onChanged();
+              }, 0);
+            }
 
             this._currentIndex++;
           }
 
           if (typeof this.onResultsAdded == "function")
             this.onResultsAdded();
 
           if (this._currentIndex < matchCount) {
@@ -1641,16 +1641,28 @@ extends="chrome://global/content/binding
           );
           this._actionText = document.getAnonymousElementByAttribute(
             this, "anonid", "action-text"
           );
           this._adjustAcItem();
         ]]>
       </constructor>
 
+      <method name="_cleanup">
+        <body>
+        <![CDATA[
+          this.removeAttribute("url");
+          this.removeAttribute("image");
+          this.removeAttribute("title");
+          this.removeAttribute("text");
+          this.removeAttribute("displayurl");
+        ]]>
+        </body>
+      </method>
+
       <property name="label" readonly="true">
         <getter>
           <![CDATA[
             // This property is a string that is read aloud by screen readers,
             // so it must not contain anything that should not be user-facing.
 
             let parts = [
               this.getAttribute("title"),
@@ -2027,19 +2039,68 @@ extends="chrome://global/content/binding
               Components.classes["@mozilla.org/intl/texttosuburi;1"]
                         .getService(Components.interfaces.nsITextToSubURI);
           }
           return this._textToSubURI.unEscapeURIForUI("UTF-8", url);
           ]]>
         </body>
       </method>
 
+      <method name="_onChanged">
+        <body>
+          <![CDATA[
+            let iconChanged = item.adjustSiteIconStart(this.popup._siteIconStart);
+
+            if (iconChanged) {
+              item.handleOverUnderflow();
+            }
+          ]]>
+        </body>
+      </method>
+
+
+      <method name="_reuseAcItem">
+        <body>
+          <![CDATA[
+            let action = this._parseActionUrl(this.getAttribute("url"));
+            let popup = this.parentNode.parentNode;
+
+            // If the item is a searchengine action, then it should
+            // only be reused if the engine name is the same as the
+            // popup's override engine name, if any.
+            if (!action ||
+                action.type != "searchengine" ||
+                !popup.overrideSearchEngineName ||
+                action.params.engineName == popup.overrideSearchEngineName) {
+
+              this.collapsed = false;
+              // Call adjustSiteIconStart only after setting collapsed=
+              // false.  The calculations it does may be wrong otherwise.
+              this.adjustSiteIconStart(popup._siteIconStart);
+              // The popup may have changed size between now and the last
+              // time the item was shown, so always handle over/underflow.
+              this.handleOverUnderflow();
+
+              return true;
+            }
+
+            return false;
+          ]]>
+        </body>
+      </method>
+
+
       <method name="_adjustAcItem">
         <body>
           <![CDATA[
+          this.setAttribute("url", this.getAttribute("ac-value"));
+          this.setAttribute("image", this.getAttribute("ac-image"));
+          this.setAttribute("title", this.getAttribute("ac-comment"));
+          this.setAttribute("text", this.getAttribute("ac-text"));
+
           let popup = this.parentNode.parentNode;
           if (!popup.popupOpen) {
             // Removing the max-width and resetting it later when overflow is
             // handled is jarring when the item is visible, so skip this when
             // the popup is open.
             this._removeMaxWidths();
           }