Bug 1535656 - Quantum bar results flicker while typing a search. r=dao
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 08 May 2019 21:14:35 +0000
changeset 531946 d6756af108596e5a27548d397f1dd72d44c45854
parent 531945 876559fca157d87c6b39f57ee4995dc09fff0db2
child 531947 f9c6e9cd8754bc3299a6c024410d8946ebea7f06
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao
bugs1535656
milestone68.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 1535656 - Quantum bar results flicker while typing a search. r=dao Differential Revision: https://phabricator.services.mozilla.com/D30190
browser/components/urlbar/UrlbarView.jsm
--- a/browser/components/urlbar/UrlbarView.jsm
+++ b/browser/components/urlbar/UrlbarView.jsm
@@ -88,17 +88,18 @@ class UrlbarView {
       throw new Error("UrlbarView: Cannot select an item if the view isn't open.");
     }
 
     if (val < 0) {
       this._selectItem(null);
       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.`);
     }
     this._selectItem(items[val]);
     return val;
   }
 
   /**
@@ -139,41 +140,50 @@ class UrlbarView {
       throw new Error("UrlbarView: Cannot select an item if the view isn't open.");
     }
 
     // Freeze results as the user is interacting with them.
     this.controller.cancelQuery();
 
     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);
       return;
     }
 
     while (amount-- > 0) {
       let next = reverse ? row.previousElementSibling : row.nextElementSibling;
       if (!next) {
         break;
       }
+      if (next.style.display == "none") {
+        continue;
+      }
       row = next;
     }
     this._selectItem(row);
   }
 
   removeAccessibleFocus() {
     this._setAccessibleFocus(null);
   }
@@ -382,30 +392,108 @@ class UrlbarView {
       verticalOffset++;
     }
     this.panel.style.marginInlineStart = px(horizontalOffset);
     this.panel.style.marginTop = px(verticalOffset);
 
     this.panel.openPopup(this.input.textbox, "after_start");
   }
 
+  /**
+   * 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;
+    }
+    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 i = 0;
-    for (let row of this._rows.children) {
-      if (i < results.length) {
-        this._updateRow(row, results[i]);
-      } else {
-        row.setAttribute("stale", "true");
+    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++;
       }
-      i++;
     }
-    for (; i < results.length; i++) {
+    // 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);
     }
   }
 
   _createRow() {
     let item = this._createElement("div");
     item.className = "urlbarView-row";
     item.setAttribute("role", "option");
@@ -547,16 +635,18 @@ class UrlbarView {
   }
 
   _removeStaleRows() {
     let row = this._rows.lastElementChild;
     while (row) {
       let next = row.previousElementSibling;
       if (row.hasAttribute("stale")) {
         row.remove();
+      } else {
+        row.style.display = "";
       }
       row = next;
     }
   }
 
   _startRemoveStaleRowsTimer() {
     this._removeStaleRowsTimer = this.window.setTimeout(() => {
       this._removeStaleRowsTimer = null;