Bug 1326138 - Refactor _appendCurrentResult to be more generic. r=adw draft
authorRay Lin <ralin@mozilla.com>
Tue, 24 Jan 2017 23:32:36 +0800
changeset 484723 88c4f611174d1db989b56924318ee5fa073aa0be
parent 484601 c0807d6938c13e43add377d5838df7168a59971e
child 484724 6f06cff951c39961dc3690797cd5e58939360445
push id45535
push userbmo:ralin@mozilla.com
push dateWed, 15 Feb 2017 17:06:22 +0000
reviewersadw
bugs1326138
milestone54.0a1
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();
           }