diff --git a/browser/components/urlbar/UrlbarView.jsm b/browser/components/urlbar/UrlbarView.jsm index 8bd377d26f83..73ae4bf886d0 100644 --- a/browser/components/urlbar/UrlbarView.jsm +++ b/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,109 +748,83 @@ class UrlbarView { this._updateIndices(); } - _createRow() { + _createRow(type) { let item = this._createElement("div"); item.className = "urlbarView-row"; item.setAttribute("role", "option"); item._elements = new Map(); + + 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"; + content.appendChild(typeIcon); + + let favicon = this._createElement("img"); + favicon.className = "urlbarView-favicon"; + content.appendChild(favicon); + item._elements.set("favicon", favicon); + + let title = this._createElement("span"); + title.className = "urlbarView-title"; + 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"; + content.appendChild(buttonSpacer); + + let tipButton = this._createElement("span"); + tipButton.className = "urlbarView-tip-button"; + tipButton.setAttribute("role", "button"); + content.appendChild(tipButton); + item._elements.set("tipButton", tipButton); + + let helpIcon = this._createElement("span"); + helpIcon.className = "urlbarView-tip-help"; + helpIcon.setAttribute("role", "button"); + helpIcon.setAttribute("data-l10n-id", "urlbar-tip-help-icon"); + item._elements.set("helpButton", 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; } - _createRowContent(item) { - let typeIcon = this._createElement("span"); - typeIcon.className = "urlbarView-type-icon"; - item._content.appendChild(typeIcon); - - 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); - - 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); - - let buttonSpacer = this._createElement("span"); - buttonSpacer.className = "urlbarView-tip-button-spacer"; - item._content.appendChild(buttonSpacer); - - let tipButton = this._createElement("span"); - tipButton.className = "urlbarView-tip-button"; - tipButton.setAttribute("role", "button"); - item._content.appendChild(tipButton); - item._elements.set("tipButton", tipButton); - - let helpIcon = this._createElement("span"); - helpIcon.className = "urlbarView-tip-help"; - helpIcon.setAttribute("role", "button"); - helpIcon.setAttribute("data-l10n-id", "urlbar-tip-help-icon"); - item._elements.set("helpButton", helpIcon); - item._content.appendChild(helpIcon); - } - _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. diff --git a/browser/components/urlbar/tests/UrlbarTestUtils.jsm b/browser/components/urlbar/tests/UrlbarTestUtils.jsm index 0ac094024e2a..f3770147956c 100644 --- a/browser/components/urlbar/tests/UrlbarTestUtils.jsm +++ b/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 - ? win.getComputedStyle(typeIcon)["background-image"] - : null, + typeIcon: typeIconStyle["background-image"], }; details.element = { action: element.getElementsByClassName("urlbarView-action")[0], diff --git a/browser/components/urlbar/tests/browser/browser.ini b/browser/components/urlbar/tests/browser/browser.ini index e0d40f3f46ae..69b8f1755908 100644 --- a/browser/components/urlbar/tests/browser/browser.ini +++ b/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 diff --git a/browser/components/urlbar/tests/browser/browser_updateRows.js b/browser/components/urlbar/tests/browser/browser_updateRows.js deleted file mode 100644 index 56a2a306adea..000000000000 --- a/browser/components/urlbar/tests/browser/browser_updateRows.js +++ /dev/null @@ -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) {} -} diff --git a/browser/themes/shared/urlbar-autocomplete.inc.css b/browser/themes/shared/urlbar-autocomplete.inc.css index a6cdb9a33cdb..5150d1f7f43f 100644 --- a/browser/themes/shared/urlbar-autocomplete.inc.css +++ b/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; }