Backed out changeset d4ccc4398453 (bug 1592172) for causing browser_updateRows.js to permafail CLOSED TREE

browser/components/urlbar/UrlbarView.jsm

@@ -735,7 +735,7 @@ class UrlbarView {
     }
     // Add remaining results, if we have fewer rows than results.
     for (; resultIndex < results.length; ++resultIndex) {
-      let row = this._createRow();
+      let row = this._createRow(results[resultIndex].type);
       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.
@@ -748,74 +748,45 @@ class UrlbarView {
     this._updateIndices();
   }
 
-  _createRow() {
+  _createRow(type) {
     let item = this._createElement("div");
     item.className = "urlbarView-row";
     item.setAttribute("role", "option");
     item._elements = new Map();
-    return item;
-  }
 
-  _createRowContent(item) {
+    let content = this._createElement("span");
+    content.className = "urlbarView-row-inner";
+    item.appendChild(content);
+    item._elements.set("rowInner", content);
+
     let typeIcon = this._createElement("span");
     typeIcon.className = "urlbarView-type-icon";
-    item._content.appendChild(typeIcon);
+    content.appendChild(typeIcon);
 
     let favicon = this._createElement("img");
     favicon.className = "urlbarView-favicon";
-    item._content.appendChild(favicon);
+    content.appendChild(favicon);
     item._elements.set("favicon", favicon);
 
     let title = this._createElement("span");
     title.className = "urlbarView-title";
-    item._content.appendChild(title);
+    content.appendChild(title);
-    item._elements.set("title", title);
-
-    let tagsContainer = this._createElement("span");
-    tagsContainer.className = "urlbarView-tags";
-    item._content.appendChild(tagsContainer);
-    item._elements.set("tagsContainer", tagsContainer);
-
-    let titleSeparator = this._createElement("span");
-    titleSeparator.className = "urlbarView-title-separator";
-    item._content.appendChild(titleSeparator);
-    item._elements.set("titleSeparator", titleSeparator);
-
-    let action = this._createElement("span");
-    action.className = "urlbarView-secondary urlbarView-action";
-    item._content.appendChild(action);
-    item._elements.set("action", action);
-
-    let url = this._createElement("span");
-    url.className = "urlbarView-secondary urlbarView-url";
-    item._content.appendChild(url);
-    item._elements.set("url", url);
-  }
-
-  _createRowContentForTip(item) {
-    // We use role="group" so screen readers will read the group's label when a
-    // button inside it gets focus. (Screen readers don't do this for
-    // role="option".) We set aria-labelledby for the group in _updateIndices.
-    item._content.setAttribute("role", "group");
-
-    let favicon = this._createElement("img");
-    favicon.className = "urlbarView-favicon";
-    item._content.appendChild(favicon);
-    item._elements.set("favicon", favicon);
-
-    let title = this._createElement("span");
-    title.className = "urlbarView-title";
-    item._content.appendChild(title);
     item._elements.set("title", title);
 
+    if (type == UrlbarUtils.RESULT_TYPE.TIP) {
+      // We use role="group" so screen readers will read the group's label
+      // when a button inside it gets focus. (Screen readers don't do this for
+      // role="option".)
+      // we set aria-labelledby for the group in _updateIndices.
+      content.setAttribute("role", "group");
       let buttonSpacer = this._createElement("span");
       buttonSpacer.className = "urlbarView-tip-button-spacer";
-    item._content.appendChild(buttonSpacer);
+      content.appendChild(buttonSpacer);
 
       let tipButton = this._createElement("span");
       tipButton.className = "urlbarView-tip-button";
       tipButton.setAttribute("role", "button");
-    item._content.appendChild(tipButton);
+      content.appendChild(tipButton);
       item._elements.set("tipButton", tipButton);
 
       let helpIcon = this._createElement("span");
@@ -823,34 +794,37 @@ class UrlbarView {
       helpIcon.setAttribute("role", "button");
       helpIcon.setAttribute("data-l10n-id", "urlbar-tip-help-icon");
       item._elements.set("helpButton", helpIcon);
-    item._content.appendChild(helpIcon);
+      // We will unhide the help icon if our payload has a helpUrl.
+      helpIcon.style.display = "none";
+      content.appendChild(helpIcon);
+    } else {
+      let tagsContainer = this._createElement("span");
+      tagsContainer.className = "urlbarView-tags";
+      content.appendChild(tagsContainer);
+      item._elements.set("tagsContainer", tagsContainer);
+
+      let titleSeparator = this._createElement("span");
+      titleSeparator.className = "urlbarView-title-separator";
+      content.appendChild(titleSeparator);
+      item._elements.set("titleSeparator", titleSeparator);
+
+      let action = this._createElement("span");
+      action.className = "urlbarView-secondary urlbarView-action";
+      content.appendChild(action);
+      item._elements.set("action", action);
+
+      let url = this._createElement("span");
+      url.className = "urlbarView-secondary urlbarView-url";
+      content.appendChild(url);
+      item._elements.set("url", url);
+    }
+    return item;
   }
 
   _updateRow(item, result) {
-    let oldResultType = item.result && item.result.type;
     item.result = result;
     item.removeAttribute("stale");
 
-    let needsNewContent =
-      oldResultType === undefined ||
-      (oldResultType == UrlbarUtils.RESULT_TYPE.TIP) !=
-        (result.type == UrlbarUtils.RESULT_TYPE.TIP);
-
-    if (needsNewContent) {
-      if (item._content) {
-        item._content.remove();
-        item._elements.clear();
-      }
-      item._content = this._createElement("span");
-      item._content.className = "urlbarView-row-inner";
-      item.appendChild(item._content);
-      if (item.result.type == UrlbarUtils.RESULT_TYPE.TIP) {
-        this._createRowContentForTip(item);
-      } else {
-        this._createRowContent(item);
-      }
-    }
-
     if (
       result.type == UrlbarUtils.RESULT_TYPE.SEARCH &&
       !result.payload.keywordOffer &&
@@ -863,8 +837,6 @@ class UrlbarView {
       item.setAttribute("type", "switchtab");
     } else if (result.type == UrlbarUtils.RESULT_TYPE.TIP) {
       item.setAttribute("type", "tip");
-      this._updateRowForTip(item, result);
-      return;
     } else if (result.source == UrlbarUtils.RESULT_SOURCE.BOOKMARKS) {
       item.setAttribute("type", "bookmark");
     } else {
@@ -877,11 +849,28 @@ class UrlbarView {
       result.type == UrlbarUtils.RESULT_TYPE.KEYWORD
     ) {
       favicon.src = result.payload.icon || UrlbarUtils.ICON.SEARCH_GLASS;
+    } else if (result.type == UrlbarUtils.RESULT_TYPE.TIP) {
+      favicon.src = result.payload.icon || UrlbarUtils.ICON.TIP;
     } else {
       favicon.src = result.payload.icon || UrlbarUtils.ICON.DEFAULT;
     }
 
     let title = item._elements.get("title");
+
+    if (result.type == UrlbarUtils.RESULT_TYPE.TIP) {
+      title.textContent = result.payload.text;
+      let tipButton = item._elements.get("tipButton");
+      tipButton.textContent = result.payload.buttonText;
+
+      if (result.payload.helpUrl) {
+        let helpIcon = item._elements.get("helpButton");
+        helpIcon.style.display = "";
+      }
+      // Tips are dissimilar to other types of results and don't need the rest
+      // of this markup. We return early.
+      return;
+    }
+
     this._addTextContentWithHighlights(
       title,
       result.title,
@@ -982,20 +971,6 @@ class UrlbarView {
     item._elements.get("titleSeparator").hidden = !action && !setURL;
   }
 
-  _updateRowForTip(item, result) {
-    let favicon = item._elements.get("favicon");
-    favicon.src = result.payload.icon || UrlbarUtils.ICON.TIP;
-
-    let title = item._elements.get("title");
-    title.textContent = result.payload.text;
-
-    let tipButton = item._elements.get("tipButton");
-    tipButton.textContent = result.payload.buttonText;
-
-    let helpIcon = item._elements.get("helpButton");
-    helpIcon.style.display = result.payload.helpUrl ? "" : "none";
-  }
-
   _updateIndices() {
     for (let i = 0; i < this._rows.children.length; i++) {
       let item = this._rows.children[i];
@@ -1004,7 +979,8 @@ class UrlbarView {
       if (item.result.type == UrlbarUtils.RESULT_TYPE.TIP) {
         let title = item._elements.get("title");
         title.id = item.id + "-title";
-        item._content.setAttribute("aria-labelledby", title.id);
+        let content = item._elements.get("rowInner");
+        content.setAttribute("aria-labelledby", title.id);
         let tipButton = item._elements.get("tipButton");
         tipButton.id = item.id + "-tip-button";
         let helpButton = item._elements.get("helpButton");
@@ -1021,7 +997,7 @@ class UrlbarView {
 
   _setRowVisibility(row, visible) {
     row.style.display = visible ? "" : "none";
-    if (!visible && row.result.type != UrlbarUtils.RESULT_TYPE.TIP) {
+    if (!visible) {
       // Reset the overflow state of elements that can overflow in case their
       // content changes while they're hidden. When making the row visible
       // again, we'll get new overflow events if needed.

browser/components/urlbar/tests/UrlbarTestUtils.jsm

@@ -138,13 +138,12 @@ var UrlbarTestUtils = {
     let actions = element.getElementsByClassName("urlbarView-action");
     let urls = element.getElementsByClassName("urlbarView-url");
     let typeIcon = element.querySelector(".urlbarView-type-icon");
+    let typeIconStyle = win.getComputedStyle(typeIcon);
     details.displayed = {
       title: element.getElementsByClassName("urlbarView-title")[0].textContent,
       action: actions.length ? actions[0].textContent : null,
       url: urls.length ? urls[0].textContent : null,
-      typeIcon: typeIcon
+      typeIcon: typeIconStyle["background-image"],
-        ? win.getComputedStyle(typeIcon)["background-image"]
-        : null,
     };
     details.element = {
       action: element.getElementsByClassName("urlbarView-action")[0],

browser/components/urlbar/tests/browser/browser.ini

@@ -113,7 +113,6 @@ support-files =
   moz.png
 [browser_textruns.js]
 [browser_tip_selection.js]
-[browser_updateRows.js]
 [browser_urlbar_blanking.js]
 support-files =
   file_blank_but_not_blank.html

browser/components/urlbar/tests/browser/browser_updateRows.js

@@ -1,263 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-// Tests row updating and reuse.
-
-"use strict";
-
-add_task(async function init() {
-  await PlacesUtils.history.clear();
-  await PlacesUtils.bookmarks.eraseEverything();
-});
-
-// A URL result is replaced with a tip result and then vice versa.
-add_task(async function urlToTip() {
-  // Add some visits that will be matched by a "test" search string.
-  await PlacesTestUtils.addVisits([
-    "http://example.com/testxx",
-    "http://example.com/test",
-  ]);
-
-  // Add a provider that returns a tip result when the search string is "testx".
-  let provider = new TestProvider([
-    new UrlbarResult(
-      UrlbarUtils.RESULT_TYPE.TIP,
-      UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL,
-      {
-        text: "This is a test tip.",
-        buttonText: "OK",
-        helpUrl: "http://example.com/",
-      }
-    ),
-  ]);
-  provider.isActive = context => context.searchString == "testx";
-  UrlbarProvidersManager.registerProvider(provider);
-
-  // Search for "test".
-  await UrlbarTestUtils.promiseAutocompleteResultPopup({
-    value: "test",
-    window,
-    waitForFocus,
-  });
-
-  // The result at index 1 should be the http://example.com/test visit.
-  await checkResult(
-    1,
-    UrlbarUtils.RESULT_TYPE.URL,
-    {
-      title: "test visit for http://example.com/test",
-      tagsContainer: null,
-      titleSeparator: null,
-      action: "",
-      url: "http://example.com/test",
-    },
-    ["tipButton", "helpButton"]
-  );
-
-  // Type an "x" so that the search string is "testx".
-  EventUtils.synthesizeKey("x");
-  await UrlbarTestUtils.promiseSearchComplete(window);
-
-  // Now the result at index 1 should be the tip from our provider.
-  await checkResult(
-    1,
-    UrlbarUtils.RESULT_TYPE.TIP,
-    {
-      title: "This is a test tip.",
-      tipButton: "OK",
-      helpButton: null,
-    },
-    ["tagsContainer", "titleSeparator", "action", "url"]
-  );
-
-  // Type another "x" so that the search string is "testxx".
-  EventUtils.synthesizeKey("x");
-  await UrlbarTestUtils.promiseSearchComplete(window);
-
-  // The result at index 1 should be the http://example.com/testxx visit.
-  await checkResult(
-    1,
-    UrlbarUtils.RESULT_TYPE.URL,
-    {
-      title: "test visit for http://example.com/testxx",
-      tagsContainer: null,
-      titleSeparator: null,
-      action: "",
-      url: "http://example.com/testxx",
-    },
-    ["tipButton", "helpButton"]
-  );
-
-  // Backspace so that the search string is "testx" again.
-  EventUtils.synthesizeKey("KEY_Backspace");
-  await UrlbarTestUtils.promiseSearchComplete(window);
-
-  // The result at index 1 should be the tip again.
-  await checkResult(
-    1,
-    UrlbarUtils.RESULT_TYPE.TIP,
-    {
-      title: "This is a test tip.",
-      tipButton: "OK",
-      helpButton: null,
-    },
-    ["tagsContainer", "titleSeparator", "action", "url"]
-  );
-
-  await UrlbarTestUtils.promisePopupClose(window, () => {
-    EventUtils.synthesizeKey("KEY_Escape");
-  });
-  UrlbarProvidersManager.unregisterProvider(provider);
-  await PlacesUtils.history.clear();
-});
-
-// A tip result is replaced with URL result and then vice versa.
-add_task(async function tipToURL() {
-  // Add a visit that will be matched by a "testx" search string.
-  await PlacesTestUtils.addVisits("http://example.com/testx");
-
-  // Add a provider that returns a tip result when the search string is "test"
-  // or "testxx".
-  let provider = new TestProvider([
-    new UrlbarResult(
-      UrlbarUtils.RESULT_TYPE.TIP,
-      UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL,
-      {
-        text: "This is a test tip.",
-        buttonText: "OK",
-        helpUrl: "http://example.com/",
-      }
-    ),
-  ]);
-  provider.isActive = context =>
-    ["test", "testxx"].includes(context.searchString);
-  UrlbarProvidersManager.registerProvider(provider);
-
-  // Search for "test".
-  await UrlbarTestUtils.promiseAutocompleteResultPopup({
-    value: "test",
-    window,
-    waitForFocus,
-  });
-
-  // The result at index 1 should be the tip from our provider.
-  await checkResult(
-    1,
-    UrlbarUtils.RESULT_TYPE.TIP,
-    {
-      title: "This is a test tip.",
-      tipButton: "OK",
-      helpButton: null,
-    },
-    ["tagsContainer", "titleSeparator", "action", "url"]
-  );
-
-  // Type an "x" so that the search string is "testx".
-  EventUtils.synthesizeKey("x");
-  await UrlbarTestUtils.promiseSearchComplete(window);
-
-  // Now the result at index 1 should be the visit.
-  await checkResult(
-    1,
-    UrlbarUtils.RESULT_TYPE.URL,
-    {
-      title: "test visit for http://example.com/testx",
-      tagsContainer: null,
-      titleSeparator: null,
-      action: "",
-      url: "http://example.com/testx",
-    },
-    ["tipButton", "helpButton"]
-  );
-
-  // Type another "x" so that the search string is "testxx".
-  EventUtils.synthesizeKey("x");
-  await UrlbarTestUtils.promiseSearchComplete(window);
-
-  // The result at index 1 should be the tip again.
-  await checkResult(
-    1,
-    UrlbarUtils.RESULT_TYPE.TIP,
-    {
-      title: "This is a test tip.",
-      tipButton: "OK",
-      helpButton: null,
-    },
-    ["tagsContainer", "titleSeparator", "action", "url"]
-  );
-
-  // Backspace so that the search string is "testx" again.
-  EventUtils.synthesizeKey("KEY_Backspace");
-  await UrlbarTestUtils.promiseSearchComplete(window);
-
-  // The result at index 1 should be the visit again.
-  await checkResult(
-    1,
-    UrlbarUtils.RESULT_TYPE.URL,
-    {
-      title: "test visit for http://example.com/testx",
-      tagsContainer: null,
-      titleSeparator: null,
-      action: "",
-      url: "http://example.com/testx",
-    },
-    ["tipButton", "helpButton"]
-  );
-
-  await UrlbarTestUtils.promisePopupClose(window, () => {
-    EventUtils.synthesizeKey("KEY_Escape");
-  });
-  UrlbarProvidersManager.unregisterProvider(provider);
-  await PlacesUtils.history.clear();
-});
-
-async function checkResult(index, type, presentElements, absentElements) {
-  let result = await UrlbarTestUtils.getDetailsOfResultAt(window, index);
-  Assert.equal(result.type, type, "Expected result type");
-
-  for (let [name, value] of Object.entries(presentElements)) {
-    let element = result.element.row._elements.get(name);
-    Assert.ok(element, `${name} should be present`);
-    if (typeof value == "string") {
-      Assert.equal(
-        element.textContent,
-        value,
-        `${name} value should be correct`
-      );
-    }
-  }
-
-  for (let name of absentElements) {
-    let element = result.element.row._elements.get(name);
-    Assert.ok(!element, `${name} should be absent`);
-  }
-}
-
-/**
- * A test provider.
- */
-class TestProvider extends UrlbarProvider {
-  constructor(results) {
-    super();
-    this._results = results;
-  }
-  get name() {
-    return "TestProvider";
-  }
-  get type() {
-    return UrlbarUtils.PROVIDER_TYPE.PROFILE;
-  }
-  isActive(context) {
-    return true;
-  }
-  isRestricting(context) {
-    return false;
-  }
-  async startQuery(context, addCallback) {
-    for (const result of this._results) {
-      addCallback(this, result);
-    }
-  }
-  cancelQuery(context) {}
-  pickResult(result) {}
-}

browser/themes/shared/urlbar-autocomplete.inc.css

@@ -189,7 +189,6 @@
 .urlbarView-row[type=tip] > .urlbarView-row-inner > .urlbarView-favicon {
   min-width: 24px;
   height: 24px;
-  margin-inline-start: calc(12px + @urlbarViewIconMarginEnd@);
   margin-inline-end: 12px;
 }