Bug 1535656 - Quantum bar results flicker while typing a search. r=dao

Differential Revision: https://phabricator.services.mozilla.com/D30190

--HG--
extra : moz-landing-system : lando

browser/components/urlbar/UrlbarView.jsm

@@ -93,7 +93,8 @@ class UrlbarView {
       return val;
     }
 
-    let items = this._rows.children;
+    let items = Array.from(this._rows.children)
+                     .filter(r => r.style.display != "none");
     if (val >= items.length) {
       throw new Error(`UrlbarView: Index ${val} is out of bounds.`);
     }
@@ -144,20 +145,26 @@ class UrlbarView {
 
     let row = this._selected;
 
+    // Results over maxResults may be hidden and should not be selectable.
+    let lastElementChild = this._rows.lastElementChild;
+    while (lastElementChild && lastElementChild.style.display == "none") {
+      lastElementChild = lastElementChild.previousElementSibling;
+    }
+
     if (!row) {
-      this._selectItem(reverse ? this._rows.lastElementChild :
+      this._selectItem(reverse ? lastElementChild :
                                  this._rows.firstElementChild);
       return;
     }
 
     let endReached = reverse ?
       (row == this._rows.firstElementChild) :
-      (row == this._rows.lastElementChild);
+      (row == lastElementChild);
     if (endReached) {
       if (this.allowEmptySelection) {
         row = null;
       } else {
-        row = reverse ? this._rows.lastElementChild :
+        row = reverse ? lastElementChild :
                         this._rows.firstElementChild;
       }
       this._selectItem(row);
@@ -169,6 +176,9 @@ class UrlbarView {
       if (!next) {
         break;
       }
+      if (next.style.display == "none") {
+        continue;
+      }
       row = next;
     }
     this._selectItem(row);
@@ -387,20 +397,98 @@ class UrlbarView {
     this.panel.openPopup(this.input.textbox, "after_start");
   }
 
-  _updateResults(queryContext) {
-    let results = queryContext.results;
-    let i = 0;
-    for (let row of this._rows.children) {
-      if (i < results.length) {
-        this._updateRow(row, results[i]);
-      } else {
-        row.setAttribute("stale", "true");
-      }
-      i++;
+  /**
+   * Whether a result is a search suggestion.
+   * @param {UrlbarResult} result The result to examine.
+   * @returns {boolean} Whether the result is a search suggestion.
+   */
+  _resultIsSearchSuggestion(result) {
+    return Boolean(result &&
+                   result.type == UrlbarUtils.RESULT_TYPE.SEARCH &&
+                   result.payload.suggestion);
+  }
+
+  /**
+   * Checks whether the given row index can be update to the result we want
+   * to apply. This is used in _updateResults to avoid flickering of results, by
+   * reusing existing rows.
+   * @param {number} rowIndex Index of the row to examine.
+   * @param {UrlbarResult} result The result we'd like to apply.
+   * @param {number} firstSearchSuggestionIndex Index of the first search suggestion.
+   * @param {number} lastSearchSuggestionIndex Index of the last search suggestion.
+   * @returns {boolean} Whether the row can be updated to this result.
+   */
+  _rowCanUpdateToResult(rowIndex, result, firstSearchSuggestionIndex,
+                        lastSearchSuggestionIndex) {
+    // The heuristic result must always be current, thus it's always compatible.
+    if (result.heuristic) {
+      return true;
     }
-    for (; i < results.length; i++) {
+    let row = this._rows.children[rowIndex];
+    let resultIsSearchSuggestion = this._resultIsSearchSuggestion(result);
+    // If the row is same type, just update it.
+    if (resultIsSearchSuggestion == this._resultIsSearchSuggestion(row.result)) {
+      return true;
+    }
+    // If the row has a different type, update it if we are in a compatible
+    // index range.
+    // In practice we don't want to overwrite a search suggestion with a non
+    // search suggestion, but we allow the opposite.
+    return resultIsSearchSuggestion && rowIndex >= firstSearchSuggestionIndex;
+  }
+
+  _updateResults(queryContext) {
+    // TODO: For now this just compares search suggestions to the rest, in the
+    // future we should make it support any type of result. Or, even better,
+    // results should be grouped, thus we can directly update groups.
+
+    // Find where are existing search suggestions.
+    let firstSearchSuggestionIndex = -1;
+    let lastSearchSuggestionIndex = -1;
+    for (let i = 0; i < this._rows.children.length; ++i) {
+      let row = this._rows.children[i];
+      // Mark every row as stale, _updateRow will unmark them later.
+      row.setAttribute("stale", "true");
+      // Skip any row that isn't a search suggestion, or is non-visible because
+      // over maxResults.
+      if (row.result.heuristic ||
+          i >= queryContext.maxResults ||
+          !this._resultIsSearchSuggestion(row.result)) {
+        continue;
+      }
+      if (firstSearchSuggestionIndex == -1) {
+        firstSearchSuggestionIndex = i;
+      }
+      lastSearchSuggestionIndex = i;
+    }
+
+    // Walk rows and find an insertion index for results. To avoid flicker, we
+    // skip rows until we find one compatible with the result we want to apply.
+    // If we couldn't find a compatible range, we'll just update.
+    let results = queryContext.results;
+    let resultIndex = 0;
+    // We can have more rows than the visible ones.
+    for (let rowIndex = 0;
+         rowIndex < this._rows.children.length && resultIndex < results.length;
+         ++rowIndex) {
+      let row = this._rows.children[rowIndex];
+      let result = results[resultIndex];
+      if (this._rowCanUpdateToResult(rowIndex, result,
+                                     firstSearchSuggestionIndex,
+                                     lastSearchSuggestionIndex)) {
+        this._updateRow(row, result);
+        resultIndex++;
+      }
+    }
+    // Add remaining results, if we have fewer rows than results.
+    for (; resultIndex < results.length; ++resultIndex) {
       let row = this._createRow();
-      this._updateRow(row, results[i]);
+      this._updateRow(row, results[resultIndex]);
+      // Due to stale rows, we may have more rows than maxResults, thus we must
+      // hide them, and we'll revert this when stale rows are removed.
+      if (this._rows.children.length >= queryContext.maxResults) {
+        row.style.display = "none";
+      }
       this._rows.appendChild(row);
     }
   }
@@ -552,6 +640,8 @@ class UrlbarView {
       let next = row.previousElementSibling;
       if (row.hasAttribute("stale")) {
         row.remove();
+      } else {
+        row.style.display = "";
       }
       row = next;
     }