From 43b648c36bdb1906a9d1c93730f9c3aeb05d98fd Mon Sep 17 00:00:00 2001 From: Drew Willcoxon Date: Tue, 6 Dec 2022 18:43:49 -0500 Subject: [PATCH] Bug 1803873 - Support row buttons in all row types and make changes to tip rows. r=dao This makes a couple of large changes: (1) "Generic" buttons (the ones added by `UrlbarView.#addRowButton()`) are now supported in all row types. The help button that's currently included in some types of rows when `result.payload.helpUrl` is defined is now supported for all row types, and two additional button types are now supported too: block buttons and labeled buttons. A row will get a block button if its `result.payload.isBlockable` is defined. It will get a labeled button if `result.payload.buttons` is defined and non-empty. A button can include a `url` property that is then added as an attribute on the button's element, and `UrlbarInput.pickResult()` will use this attribute to load the URL when the button is picked. (2) The reason I added labeled buttons is because it lets us support tip buttons without much more effort, which then lets us get rid of the special row type used for tips. With this patch, tips are now standard rows that use generic buttons. This approach should be compatible with the result menu, when we switch over to it, because we can include the help and block commands in the menu when `helpUrl` and `isBlockable` are defined, instead of creating buttons for them. Labeled buttons -- the ones used in tips -- would still be created. The result menu button itself can continue to be a generic button. It should also be compatible with including the result menu button inside the row selection. We'll still add buttons to `.urlbarView-row`, separate from `.urlbarView-row-inner`, so that the buttons can continue to be on the right side of the row. We can color the background of the row instead of the row-inner. As with D163630, my motivation for this change is to support generic buttons in dynamic result rows so that help and block buttons can be easily added to weather suggestions. Here too the larger changes of supporting generic labeled buttons and removing special rows for tips aren't strictly necessary, but I took the opportunity to rework things. Finally, this makes a few other changes: * It includes some of the more minor improvements to selection that I made in D163630. * It removes the help URL code from quick actions since it was decided not to show a help button. Currently, the button is hidden in CSS, but now that a generic help button is added for dynamic result rows when `result.payload.helpUrl` is defined, `helpUrl` needs to be removed from the payload to prevent a button from being added. * I removed the special tip wrapping behavior, where the tip button and help button would wrap below the tip's text. Instead, now the text wraps inside row-inner and the buttons always remain on the same horizontal as the text. I don't think it's worth the extra complication. Differential Revision: https://phabricator.services.mozilla.com/D163766 --- .../events/browser_test_focus_urlbar.js | 17 +- .../test/browser/browser_ext_urlbar.js | 4 +- browser/components/urlbar/UrlbarInput.sys.mjs | 113 +++--- .../UrlbarProviderInterventions.sys.mjs | 44 +-- .../urlbar/UrlbarProviderQuickActions.sys.mjs | 20 -- .../urlbar/UrlbarProviderQuickSuggest.sys.mjs | 8 +- .../urlbar/UrlbarProviderSearchTips.sys.mjs | 21 +- browser/components/urlbar/UrlbarUtils.sys.mjs | 79 ++++- browser/components/urlbar/UrlbarView.sys.mjs | 325 +++++++----------- browser/components/urlbar/docs/overview.rst | 4 +- .../browser_glean_telemetry_engagement.js | 12 +- .../browser-tips/browser_interventions.js | 17 +- .../tests/browser-tips/browser_picks.js | 55 ++- .../browser_searchTips_interaction.js | 8 +- .../tests/browser-tips/browser_selection.js | 47 +-- .../urlbar/tests/browser-tips/head.js | 42 ++- .../tests/browser-updateResults/browser.ini | 1 - .../browser-updateResults/browser_reuse.js | 235 ------------- .../urlbar/tests/browser/browser_bestMatch.js | 156 +++------ .../urlbar/tests/browser/browser_helpUrl.js | 10 +- .../browser/browser_heuristicNotAddedFirst.js | 40 +-- .../tests/browser/browser_resultSpan.js | 55 ++- .../browser/browser_result_onSelection.js | 11 +- .../browser/browser_suppressFocusBorder.js | 2 +- ...owser_urlbar_event_telemetry_engagement.js | 11 +- .../browser/browser_urlbar_telemetry_tip.js | 11 +- .../urlbar/tests/browser/head-glean.js | 2 +- .../quicksuggest/unit/test_quicksuggest.js | 20 +- .../unit/test_quicksuggest_bestMatch.js | 12 +- .../unit/test_quicksuggest_impressionCaps.js | 8 +- .../unit/test_quicksuggest_merino.js | 16 +- .../test_quicksuggest_nonUniqueKeywords.js | 4 +- ...test_quicksuggest_positionInSuggestions.js | 4 +- .../tests/quicksuggest/unit/test_weather.js | 2 +- .../themes/shared/urlbar-dynamic-results.css | 4 +- browser/themes/shared/urlbarView.css | 158 ++++----- 36 files changed, 588 insertions(+), 990 deletions(-) delete mode 100644 browser/components/urlbar/tests/browser-updateResults/browser_reuse.js diff --git a/accessible/tests/browser/events/browser_test_focus_urlbar.js b/accessible/tests/browser/events/browser_test_focus_urlbar.js index 77fde86fae70..7ea68e09788e 100644 --- a/accessible/tests/browser/events/browser_test_focus_urlbar.js +++ b/accessible/tests/browser/events/browser_test_focus_urlbar.js @@ -50,9 +50,7 @@ function isEventForTipButton(event) { let parent = event.accessible.parent; return ( event.accessible.role == ROLE_PUSHBUTTON && - parent && - parent.role == ROLE_GROUPING && - parent.name + parent?.role == ROLE_COMBOBOX_LIST ); } @@ -333,12 +331,15 @@ async function runTipTests() { UrlbarUtils.RESULT_TYPE.TIP, UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, { - icon: "", - text: "This is a test intervention.", - buttonText: "Done", + helpUrl: "http://example.com/", type: "test", - helpUrl: "about:blank", - buttonUrl: "about:mozilla", + titleL10n: { id: "urlbar-search-tips-confirm" }, + buttons: [ + { + url: "http://example.com/", + l10n: { id: "urlbar-search-tips-confirm" }, + }, + ], } ), new UrlbarResult( diff --git a/browser/components/extensions/test/browser/browser_ext_urlbar.js b/browser/components/extensions/test/browser/browser_ext_urlbar.js index 7b3a541fdeea..baec7a745ee6 100644 --- a/browser/components/extensions/test/browser/browser_ext_urlbar.js +++ b/browser/components/extensions/test/browser/browser_ext_urlbar.js @@ -124,7 +124,7 @@ add_task(async function tip_onResultPicked_mainButton_noURL_mouse() { waitForFocus, value: "test", }); - let mainButton = gURLBar.querySelector(".urlbarView-tip-button"); + let mainButton = gURLBar.querySelector(".urlbarView-button-tip"); Assert.ok(mainButton); EventUtils.synthesizeMouseAtCenter(mainButton, {}); await ext.awaitMessage("onResultPicked received"); @@ -163,7 +163,7 @@ add_task(async function tip_onResultPicked_mainButton_url_mouse() { waitForFocus, value: "test", }); - let mainButton = gURLBar.querySelector(".urlbarView-tip-button"); + let mainButton = gURLBar.querySelector(".urlbarView-button-tip"); Assert.ok(mainButton); let loadedPromise = BrowserTestUtils.browserLoaded( gBrowser.selectedBrowser diff --git a/browser/components/urlbar/UrlbarInput.sys.mjs b/browser/components/urlbar/UrlbarInput.sys.mjs index 9e804d8d63c8..7c4f8976c276 100644 --- a/browser/components/urlbar/UrlbarInput.sys.mjs +++ b/browser/components/urlbar/UrlbarInput.sys.mjs @@ -835,11 +835,7 @@ export class UrlbarInput { return; } - let urlOverride; - if (element?.classList.contains("urlbarView-button-help")) { - urlOverride = result.payload.helpUrl; - } - + let urlOverride = element?.dataset.url; let originalUntrimmedValue = this.untrimmedValue; let isCanonized = this.setValueFromResult({ result, event, urlOverride }); let where = this._whereToOpen(event); @@ -1034,74 +1030,57 @@ export class UrlbarInput { break; } case lazy.UrlbarUtils.RESULT_TYPE.TIP: { - let scalarName; - if (element.classList.contains("urlbarView-button-help")) { - url = result.payload.helpUrl; - if (!url) { - Cu.reportError("helpUrl not specified"); - return; - } - scalarName = `${result.payload.type}-help`; - } else { - scalarName = `${result.payload.type}-picked`; - } + let scalarName = + element.dataset.name == "help" + ? `${result.payload.type}-help` + : `${result.payload.type}-picked`; Services.telemetry.keyedScalarAdd("urlbar.tips", scalarName, 1); - if (!url) { - this.handleRevert(); - this.controller.engagementEvent.record(event, { - searchString: this._lastSearchString, - selIndex, - selType: "tip", - provider: result.providerName, - }); - let provider = lazy.UrlbarProvidersManager.getProvider( - result.providerName - ); - if (!provider) { - Cu.reportError(`Provider not found: ${result.providerName}`); - return; - } - provider.tryMethod("pickResult", result, element); - return; + if (url) { + break; } - break; + this.handleRevert(); + this.controller.engagementEvent.record(event, { + searchString: this._lastSearchString, + selIndex, + selType: "tip", + provider: result.providerName, + }); + let provider = lazy.UrlbarProvidersManager.getProvider( + result.providerName + ); + provider?.tryMethod("pickResult", result, element); + return; } case lazy.UrlbarUtils.RESULT_TYPE.DYNAMIC: { - if (element.classList.contains("urlbarView-button-help")) { - url = result.payload.helpUrl; - } else { - url = result.payload.url; - // Do not revert the Urlbar if we're going to navigate. We want the URL - // populated so we can navigate to it. - if (!url || !result.payload.shouldNavigate) { - this.handleRevert(); - } - let provider = lazy.UrlbarProvidersManager.getProvider( - result.providerName - ); - if (!provider) { - Cu.reportError(`Provider not found: ${result.providerName}`); - return; - } + if (url) { + break; + } + url = result.payload.url; + // Do not revert the Urlbar if we're going to navigate. We want the URL + // populated so we can navigate to it. + if (!url || !result.payload.shouldNavigate) { + this.handleRevert(); + } + let provider = lazy.UrlbarProvidersManager.getProvider( + result.providerName + ); - // Keep startEventInfo since the startEventInfo state might be changed - // if the URL Bar loses focus on pickResult. - const startEventInfo = this.controller.engagementEvent - ._startEventInfo; - provider.tryMethod("pickResult", result, element); + // Keep startEventInfo since the startEventInfo state might be changed + // if the URL Bar loses focus on pickResult. + const startEventInfo = this.controller.engagementEvent._startEventInfo; + provider?.tryMethod("pickResult", result, element); - // If we won't be navigating, this is the end of the engagement. - if (!url || !result.payload.shouldNavigate) { - this.controller.engagementEvent.record(event, { - selIndex, - searchString: this._lastSearchString, - selType: this.controller.engagementEvent.typeFromElement(element), - provider: result.providerName, - element, - startEventInfo, - }); - return; - } + // If we won't be navigating, this is the end of the engagement. + if (!url || !result.payload.shouldNavigate) { + this.controller.engagementEvent.record(event, { + selIndex, + searchString: this._lastSearchString, + selType: this.controller.engagementEvent.typeFromElement(element), + provider: result.providerName, + element, + startEventInfo, + }); + return; } break; } diff --git a/browser/components/urlbar/UrlbarProviderInterventions.sys.mjs b/browser/components/urlbar/UrlbarProviderInterventions.sys.mjs index fb1b9355851c..75ad3bcf2019 100644 --- a/browser/components/urlbar/UrlbarProviderInterventions.sys.mjs +++ b/browser/components/urlbar/UrlbarProviderInterventions.sys.mjs @@ -383,48 +383,48 @@ export class QueryScorer { } /** - * Gets appropriate l10n values for each tip's payload. + * Gets appropriate values for each tip's payload. * * @param {string} tip a value from the TIPS enum - * @returns {object} an Object shaped as { textData, buttonTextData, helpUrl } + * @returns {object} Properties to include in the payload */ -function getL10nPropertiesForTip(tip) { - const baseURL = "https://support.mozilla.org/kb/"; +function getPayloadForTip(tip) { + const baseURL = Services.urlFormatter.formatURLPref("app.support.baseURL"); switch (tip) { case TIPS.CLEAR: return { - textData: { id: "intervention-clear-data" }, - buttonTextData: { id: "intervention-clear-data-confirm" }, + titleL10n: { id: "intervention-clear-data" }, + buttons: [{ l10n: { id: "intervention-clear-data-confirm" } }], helpUrl: baseURL + "delete-browsing-search-download-history-firefox", }; case TIPS.REFRESH: return { - textData: { id: "intervention-refresh-profile" }, - buttonTextData: { id: "intervention-refresh-profile-confirm" }, + titleL10n: { id: "intervention-refresh-profile" }, + buttons: [{ l10n: { id: "intervention-refresh-profile-confirm" } }], helpUrl: baseURL + "refresh-firefox-reset-add-ons-and-settings", }; case TIPS.UPDATE_ASK: return { - textData: { id: "intervention-update-ask" }, - buttonTextData: { id: "intervention-update-ask-confirm" }, + titleL10n: { id: "intervention-update-ask" }, + buttons: [{ l10n: { id: "intervention-update-ask-confirm" } }], helpUrl: baseURL + "update-firefox-latest-release", }; case TIPS.UPDATE_REFRESH: return { - textData: { id: "intervention-update-refresh" }, - buttonTextData: { id: "intervention-update-refresh-confirm" }, + titleL10n: { id: "intervention-update-refresh" }, + buttons: [{ l10n: { id: "intervention-update-refresh-confirm" } }], helpUrl: baseURL + "refresh-firefox-reset-add-ons-and-settings", }; case TIPS.UPDATE_RESTART: return { - textData: { id: "intervention-update-restart" }, - buttonTextData: { id: "intervention-update-restart-confirm" }, + titleL10n: { id: "intervention-update-restart" }, + buttons: [{ l10n: { id: "intervention-update-restart-confirm" } }], helpUrl: baseURL + "update-firefox-latest-release", }; case TIPS.UPDATE_WEB: return { - textData: { id: "intervention-update-web" }, - buttonTextData: { id: "intervention-update-web-confirm" }, + titleL10n: { id: "intervention-update-web" }, + buttons: [{ l10n: { id: "intervention-update-web-confirm" } }], helpUrl: baseURL + "update-firefox-latest-release", }; default: @@ -657,20 +657,14 @@ class ProviderInterventions extends UrlbarProvider { UrlbarUtils.RESULT_TYPE.TIP, UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, { + ...getPayloadForTip(this.currentTip), type: this.currentTip, + icon: UrlbarUtils.ICON.TIP, + helpL10n: { id: "urlbar-tip-help-icon" }, } ); - result.suggestedIndex = 1; - - Object.assign(result.payload, getL10nPropertiesForTip(this.currentTip)); - - if (instance != this.queryInstance) { - return; - } - this.tipsShownInCurrentEngagement.add(this.currentTip); - addCallback(this, result); } diff --git a/browser/components/urlbar/UrlbarProviderQuickActions.sys.mjs b/browser/components/urlbar/UrlbarProviderQuickActions.sys.mjs index 150544daf0d0..252e3ddf4e82 100644 --- a/browser/components/urlbar/UrlbarProviderQuickActions.sys.mjs +++ b/browser/components/urlbar/UrlbarProviderQuickActions.sys.mjs @@ -63,13 +63,6 @@ class ProviderQuickActions extends UrlbarProvider { return UrlbarUtils.PROVIDER_TYPE.PROFILE; } - get helpUrl() { - return ( - Services.urlFormatter.formatURLPref("app.support.baseURL") + - "quick-actions-firefox-search-bar" - ); - } - getPriority(context) { if (!context.searchString) { return 1; @@ -147,7 +140,6 @@ class ProviderQuickActions extends UrlbarProvider { { results: results.map(key => ({ key })), dynamicType: DYNAMIC_TYPE_NAME, - helpUrl: this.helpUrl, inputLength: input.length, } ); @@ -157,9 +149,6 @@ class ProviderQuickActions extends UrlbarProvider { getViewTemplate(result) { return { - attributes: { - selectable: false, - }, children: [ { name: "buttons", @@ -205,15 +194,6 @@ class ProviderQuickActions extends UrlbarProvider { return row; }), }, - { - name: "onboarding", - tag: "a", - attributes: { - "data-key": "onboarding-button", - class: "urlbarView-button urlbarView-button-help", - "data-l10n-id": "quickactions-learn-more", - }, - }, ], }; } diff --git a/browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs b/browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs index 364ba336e356..280b4ccc2c45 100644 --- a/browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs +++ b/browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs @@ -202,7 +202,7 @@ class ProviderQuickSuggest extends UrlbarProvider { sponsoredIabCategory: suggestion.iab_category, isSponsored: suggestion.is_sponsored, helpUrl: lazy.QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, source: suggestion.source, requestId: suggestion.request_id, }; @@ -229,10 +229,14 @@ class ProviderQuickSuggest extends UrlbarProvider { // Show the result as a best match. Best match titles don't include the // `full_keyword`, and the user's search string is highlighted. payload.title = [suggestion.title, UrlbarUtils.HIGHLIGHT.TYPED]; + payload.isBlockable = lazy.UrlbarPrefs.get("bestMatchBlockingEnabled"); + payload.blockL10n = { id: "firefox-suggest-urlbar-block" }; } else { // Show the result as a usual quick suggest. Include the `full_keyword` // and highlight the parts that aren't in the search string. payload.title = suggestion.title; + payload.isBlockable = lazy.UrlbarPrefs.get("quickSuggestBlockingEnabled"); + payload.blockL10n = { id: "firefox-suggest-urlbar-block" }; payload.qsSuggestion = [ suggestion.full_keyword, UrlbarUtils.HIGHLIGHT.SUGGESTED, @@ -646,7 +650,7 @@ class ProviderQuickSuggest extends UrlbarProvider { url: suggestion.url, icon: "chrome://global/skin/icons/highlights.svg", helpUrl: lazy.QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, requestId: suggestion.request_id, source: suggestion.source, } diff --git a/browser/components/urlbar/UrlbarProviderSearchTips.sys.mjs b/browser/components/urlbar/UrlbarProviderSearchTips.sys.mjs index 1abd2a8c23aa..6279121330d8 100644 --- a/browser/components/urlbar/UrlbarProviderSearchTips.sys.mjs +++ b/browser/components/urlbar/UrlbarProviderSearchTips.sys.mjs @@ -197,13 +197,16 @@ class ProviderSearchTips extends UrlbarProvider { this.currentTip = TIPS.NONE; let defaultEngine = await Services.search.getDefault(); + if (instance != this.queryInstance) { + return; + } let result = new lazy.UrlbarResult( UrlbarUtils.RESULT_TYPE.TIP, UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, { type: tip, - buttonTextData: { id: "urlbar-search-tips-confirm" }, + buttons: [{ l10n: { id: "urlbar-search-tips-confirm" } }], icon: defaultEngine.iconURI?.spec, } ); @@ -211,7 +214,7 @@ class ProviderSearchTips extends UrlbarProvider { switch (tip) { case TIPS.ONBOARD: result.heuristic = true; - result.payload.textData = { + result.payload.titleL10n = { id: "urlbar-search-tips-onboard", args: { engineName: defaultEngine.name, @@ -220,7 +223,7 @@ class ProviderSearchTips extends UrlbarProvider { break; case TIPS.REDIRECT: result.heuristic = false; - result.payload.textData = { + result.payload.titleL10n = { id: "urlbar-search-tips-redirect-2", args: { engineName: defaultEngine.name, @@ -229,20 +232,16 @@ class ProviderSearchTips extends UrlbarProvider { break; case TIPS.PERSIST: result.heuristic = true; - result.payload.textData = { + result.payload.titleL10n = { id: "urlbar-search-tips-persist", }; result.payload.icon = UrlbarUtils.ICON.TIP; - result.payload.buttonTextData = { - id: "urlbar-search-tips-confirm-short", - }; + result.payload.buttons = [ + { l10n: { id: "urlbar-search-tips-confirm-short" } }, + ]; break; } - if (instance != this.queryInstance) { - return; - } - Services.telemetry.keyedScalarAdd("urlbar.tips", `${tip}-shown`, 1); addCallback(this, result); diff --git a/browser/components/urlbar/UrlbarUtils.sys.mjs b/browser/components/urlbar/UrlbarUtils.sys.mjs index b3a11f5e2b94..f7dee6e41b57 100644 --- a/browser/components/urlbar/UrlbarUtils.sys.mjs +++ b/browser/components/urlbar/UrlbarUtils.sys.mjs @@ -601,11 +601,6 @@ export var UrlbarUtils = { } break; } - case UrlbarUtils.RESULT_TYPE.TIP: { - // Return the button URL. Consumers must check payload.helpUrl - // themselves if they need the tip's help link. - return { url: result.payload.buttonUrl, postData: null }; - } } return { url: null, postData: null }; }, @@ -1517,11 +1512,34 @@ UrlbarUtils.RESULT_PAYLOAD_SCHEMA = { type: "object", required: ["url"], properties: { + // l10n { id, args } + blockL10n: { + type: "object", + required: ["id"], + properties: { + id: { + type: "string", + }, + args: { + type: "array", + }, + }, + }, displayUrl: { type: "string", }, - helpL10nId: { - type: "string", + // l10n { id, args } + helpL10n: { + type: "object", + required: ["id"], + properties: { + id: { + type: "string", + }, + args: { + type: "array", + }, + }, }, helpUrl: { type: "string", @@ -1529,6 +1547,9 @@ UrlbarUtils.RESULT_PAYLOAD_SCHEMA = { icon: { type: "string", }, + isBlockable: { + type: "boolean", + }, isPinned: { type: "boolean", }, @@ -1661,13 +1682,42 @@ UrlbarUtils.RESULT_PAYLOAD_SCHEMA = { type: "object", required: ["type"], properties: { - // Prefer `buttonTextData` if your string is translated. This is for - // untranslated strings. + buttons: { + type: "array", + items: { + type: "object", + required: ["l10n"], + properties: { + l10n: { + type: "object", + required: ["id"], + properties: { + id: { + type: "string", + }, + args: { + type: "array", + }, + }, + }, + url: { + type: "string", + }, + }, + }, + }, + // TODO: This is intended only for WebExtensions. We should remove it and + // the WebExtensions urlbar API since we're no longer using it. buttonText: { type: "string", }, + // TODO: This is intended only for WebExtensions. We should remove it and + // the WebExtensions urlbar API since we're no longer using it. + buttonUrl: { + type: "string", + }, // l10n { id, args } - buttonTextData: { + helpL10n: { type: "object", required: ["id"], properties: { @@ -1679,22 +1729,19 @@ UrlbarUtils.RESULT_PAYLOAD_SCHEMA = { }, }, }, - buttonUrl: { - type: "string", - }, helpUrl: { type: "string", }, icon: { type: "string", }, - // Prefer `text` if your string is translated. This is for untranslated - // strings. + // TODO: This is intended only for WebExtensions. We should remove it and + // the WebExtensions urlbar API since we're no longer using it. text: { type: "string", }, // l10n { id, args } - textData: { + titleL10n: { type: "object", required: ["id"], properties: { diff --git a/browser/components/urlbar/UrlbarView.sys.mjs b/browser/components/urlbar/UrlbarView.sys.mjs index 0da14133010d..5b80ce5113d4 100644 --- a/browser/components/urlbar/UrlbarView.sys.mjs +++ b/browser/components/urlbar/UrlbarView.sys.mjs @@ -31,9 +31,6 @@ XPCOMUtils.defineLazyServiceGetter( // Query selector for selectable elements in tip and dynamic results. const SELECTABLE_ELEMENT_SELECTOR = "[role=button], [selectable=true]"; -// Query selector that prevents rows from being selectable. -const UNSELECTABLE_ELEMENT_SELECTOR = "[selectable=false]"; - const getBoundsWithoutFlushing = element => element.ownerGlobal.windowUtils.getBoundsWithoutFlushing(element); @@ -152,7 +149,16 @@ export class UrlbarView { if (val >= items.length) { throw new Error(`UrlbarView: Index ${val} is out of bounds.`); } - this.#selectElement(items[val]); + + // If the row itself isn't selectable, select the first element inside it + // that is. If it doesn't contain a selectable element, clear the selection. + let element = items[val]; + if (!this.#isSelectableElement(element)) { + let next = this.#getNextSelectableElement(element); + element = this.#getRowFromElement(next) == element ? next : null; + } + + this.#selectElement(element); } get selectedElementIndex() { @@ -258,17 +264,7 @@ export class UrlbarView { * The result of the element's row. */ getResultFromElement(element) { - if (!this.isOpen) { - return null; - } - - let row = this.#getRowFromElement(element); - - if (!row) { - return null; - } - - return row.result; + return this.#getRowFromElement(element)?.result; } /** @@ -1159,66 +1155,6 @@ export class UrlbarView { url.className = "urlbarView-url"; item._content.appendChild(url); item._elements.set("url", url); - - // Usually we create all child elements for the row regardless of whether - // the specific result will use them, but we don't expect the vast majority - // of rows to need help or block buttons, so as an optimization, create them - // only when necessary. - if ( - result.providerName == "UrlbarProviderQuickSuggest" && - lazy.UrlbarPrefs.get("quickSuggestBlockingEnabled") - ) { - this.#addRowButton(item, "block", "firefox-suggest-urlbar-block"); - } - if (result.payload.helpUrl) { - this.#addRowButton(item, "help", result.payload.helpL10nId); - } - if (lazy.UrlbarPrefs.get("resultMenu")) { - this.#addRowButton(item, "menu", "urlbar-result-menu-button"); - } - } - - #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 #updateRowForTip. - item._content.setAttribute("role", "group"); - - let favicon = this.#createElement("img"); - favicon.className = "urlbarView-favicon"; - favicon.setAttribute("data-l10n-id", "urlbar-tip-icon-description"); - 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.classList.add("urlbarView-button", "urlbarView-button-help"); - helpIcon.setAttribute("role", "button"); - helpIcon.setAttribute("data-l10n-id", "urlbar-tip-help-icon"); - item._elements.set("helpButton", helpIcon); - item._content.appendChild(helpIcon); - - // Due to role=button, the button and help icon can sometimes become - // focused. We want to prevent that because the input should always be - // focused instead. (This happens when input.search("", { focus: false }) - // is called, a tip is the first result but not heuristic, and the user tabs - // the into the button from the navbar buttons. The input is skipped and - // the focus goes straight to the tip button.) - item.addEventListener("focus", () => this.input.focus(), true); } #createRowContentForDynamicType(item, result) { @@ -1310,16 +1246,48 @@ export class UrlbarView { bottom.className = "urlbarView-row-body-bottom"; body.appendChild(bottom); item._elements.set("bottom", bottom); + } - if (lazy.UrlbarPrefs.get("bestMatchBlockingEnabled")) { - this.#addRowButton(item, "block", "firefox-suggest-urlbar-block"); + #addRowButtons(item, result) { + for (let i = 0; i < result.payload.buttons?.length; i++) { + this.#addRowButton(item, { + name: i.toString(), + ...result.payload.buttons[i], + }); + } + + // TODO: `buttonText` is intended only for WebExtensions. We should remove + // it and the WebExtensions urlbar API since we're no longer using it. + if (result.payload.buttonText) { + this.#addRowButton(item, { + name: "tip", + url: result.payload.buttonUrl, + }); + item._buttons.get("tip").textContent = result.payload.buttonText; + } + + if (result.payload.isBlockable) { + this.#addRowButton(item, { + name: "block", + l10n: result.payload.blockL10n, + }); } if (result.payload.helpUrl) { - this.#addRowButton(item, "help", result.payload.helpL10nId); + this.#addRowButton(item, { + name: "help", + url: result.payload.helpUrl, + l10n: result.payload.helpL10n, + }); + } + if (lazy.UrlbarPrefs.get("resultMenu")) { + this.#addRowButton(item, { + name: "menu", + l10n: { id: "urlbar-result-menu-button" }, + }); } } - #addRowButton(item, name, l10nID) { + #addRowButton(item, { name, l10n, url }) { if (!item._buttons.size) { // If the content is marked as selectable, the screen reader will not be // able to read the text directly child of the "urlbarView-row". As the @@ -1331,15 +1299,26 @@ export class UrlbarView { item._content.insertBefore(groupAriaLabel, item._content.firstChild); item._elements.set("groupAriaLabel", groupAriaLabel); + // When the row has buttons we set role=option on row-inner since the + // latter is the selectable logical row element when buttons are present. + // Since row-inner is not a child of the role=listbox element (the row + // container, this.#rows), screen readers will not automatically recognize + // it as a listbox option. To compensate, set role=presentation on the row + // so that screen readers ignore it. + item.setAttribute("role", "presentation"); item._content.setAttribute("role", "option"); - item._content.setAttribute("selectable", "true"); } let button = this.#createElement("span"); + button.id = `${item.id}-button-${name}`; button.classList.add("urlbarView-button", "urlbarView-button-" + name); button.setAttribute("role", "button"); - if (l10nID) { - button.setAttribute("data-l10n-id", l10nID); + button.dataset.name = name; + if (l10n) { + this.#setElementL10n(button, l10n); + } + if (url) { + button.dataset.url = url; } item._buttons.set(name, button); item.appendChild(button); @@ -1356,8 +1335,6 @@ export class UrlbarView { let needsNewContent = oldResultType === undefined || - (oldResultType == lazy.UrlbarUtils.RESULT_TYPE.TIP) != - (result.type == lazy.UrlbarUtils.RESULT_TYPE.TIP) || (oldResultType == lazy.UrlbarUtils.RESULT_TYPE.DYNAMIC) != (result.type == lazy.UrlbarUtils.RESULT_TYPE.DYNAMIC) || (oldResultType == lazy.UrlbarUtils.RESULT_TYPE.DYNAMIC && @@ -1368,13 +1345,11 @@ export class UrlbarView { provider.getViewTemplate || oldResult.isBestMatch != result.isBestMatch || !!result.payload.helpUrl != item._buttons.has("help") || - (result.isBestMatch && - lazy.UrlbarPrefs.get("bestMatchBlockingEnabled") != - item._buttons.has("block")) || - (!result.isBestMatch && - result.providerName == "UrlbarProviderQuickSuggest" && - lazy.UrlbarPrefs.get("quickSuggestBlockingEnabled") != - item._buttons.has("block")); + !!result.payload.isBlockable != item._buttons.has("block") || + !lazy.ObjectUtils.deepEqual( + oldResult.payload.buttons, + result.payload.buttons + ); if (needsNewContent) { while (item.lastChild) { @@ -1386,15 +1361,14 @@ export class UrlbarView { item._content.className = "urlbarView-row-inner"; item.appendChild(item._content); item.removeAttribute("dynamicType"); - if (item.result.type == lazy.UrlbarUtils.RESULT_TYPE.TIP) { - this.#createRowContentForTip(item); - } else if (item.result.type == lazy.UrlbarUtils.RESULT_TYPE.DYNAMIC) { + if (item.result.type == lazy.UrlbarUtils.RESULT_TYPE.DYNAMIC) { this.#createRowContentForDynamicType(item, result); } else if (item.result.isBestMatch) { this.#createRowContentForBestMatch(item, result); } else { this.#createRowContent(item, result); } + this.#addRowButtons(item, result); } item._content.id = item.id + "-inner"; @@ -1410,8 +1384,23 @@ export class UrlbarView { item.setAttribute("type", "switchtab"); } else if (result.type == lazy.UrlbarUtils.RESULT_TYPE.TIP) { item.setAttribute("type", "tip"); - this.#updateRowForTip(item, result); - return; + + // Due to role=button, the button and help icon can sometimes become + // focused. We want to prevent that because the input should always be + // focused instead. (This happens when input.search("", { focus: false }) + // is called, a tip is the first result but not heuristic, and the user + // tabs the into the button from the navbar buttons. The input is skipped + // and the focus goes straight to the tip button.) + item.addEventListener("focus", () => this.input.focus(), true); + + if (result.providerName == "UrlbarProviderSearchTips") { + // For a11y, we treat search tips as alerts. We use A11yUtils.announce + // instead of role="alert" because role="alert" will only fire an alert + // event when the alert (or something inside it) is the root of an + // insertion. In this case, the entire tip result gets inserted into the + // a11y tree as a single insertion, so no alert event would be fired. + this.window.A11yUtils.announce(result.payload.titleL10n); + } } else if (result.source == lazy.UrlbarUtils.RESULT_SOURCE.BOOKMARKS) { item.setAttribute("type", "bookmark"); } else if (result.type == lazy.UrlbarUtils.RESULT_TYPE.DYNAMIC) { @@ -1476,6 +1465,7 @@ export class UrlbarView { let actionSetter = null; let isVisitAction = false; let setURL = false; + let isRowSelectable = true; switch (result.type) { case lazy.UrlbarUtils.RESULT_TYPE.TAB_SWITCH: actionSetter = () => { @@ -1535,6 +1525,9 @@ export class UrlbarView { action.textContent = result.payload.content; }; break; + case lazy.UrlbarUtils.RESULT_TYPE.TIP: + isRowSelectable = false; + break; default: if (result.heuristic && !result.autofill?.hasTitle) { isVisitAction = true; @@ -1544,6 +1537,14 @@ export class UrlbarView { break; } + if (isRowSelectable) { + let selectableElement = item._buttons.size ? item._content : item; + selectableElement.setAttribute("selectable", "true"); + } else { + item.removeAttribute("selectable"); + item._content.removeAttribute("selectable"); + } + if (result.providerName == "TabToSearch") { action.toggleAttribute("slide-in", true); } else { @@ -1626,8 +1627,6 @@ export class UrlbarView { } else { title.removeAttribute("dir"); } - - this.#updateRowForButtons(item); } #iconForResult(result, iconUrlOverride = null) { @@ -1645,47 +1644,6 @@ export class UrlbarView { ); } - #updateRowForTip(item, result) { - let favicon = item._elements.get("favicon"); - favicon.src = result.payload.icon || lazy.UrlbarUtils.ICON.TIP; - favicon.id = item.id + "-icon"; - - let title = item._elements.get("title"); - title.id = item.id + "-title"; - // Add-ons will provide text, rather than l10n ids. - if (result.payload.textData) { - this.#l10nCache.ensureAll([result.payload.textData]); - this.#setElementL10n(title, result.payload.textData); - } else { - title.textContent = result.payload.text; - } - - item._content.setAttribute("aria-labelledby", `${favicon.id} ${title.id}`); - - let tipButton = item._elements.get("tipButton"); - tipButton.id = item.id + "-tip-button"; - // Add-ons will provide buttonText, rather than l10n ids. - if (result.payload.buttonTextData) { - this.#l10nCache.ensureAll([result.payload.buttonTextData]); - this.#setElementL10n(tipButton, result.payload.buttonTextData); - } else { - tipButton.textContent = result.payload.buttonText; - } - - let helpIcon = item._elements.get("helpButton"); - helpIcon.id = item.id + "-tip-help"; - helpIcon.style.display = result.payload.helpUrl ? "" : "none"; - - if (result.providerName == "UrlbarProviderSearchTips") { - // For a11y, we treat search tips as alerts. We use A11yUtils.announce - // instead of role="alert" because role="alert" will only fire an alert - // event when the alert (or something inside it) is the root of an - // insertion. In this case, the entire tip result gets inserted into the - // a11y tree as a single insertion, so no alert event would be fired. - this.window.A11yUtils.announce(result.payload.textData); - } - } - async #updateRowForDynamicType(item, result) { item.setAttribute("dynamicType", result.payload.dynamicType); @@ -1754,6 +1712,9 @@ export class UrlbarView { } #updateRowForBestMatch(item, result) { + let selectableElement = item._buttons.size ? item._content : item; + selectableElement.setAttribute("selectable", "true"); + let favicon = item._elements.get("favicon"); favicon.src = this.#iconForResult(result); @@ -1785,30 +1746,6 @@ export class UrlbarView { } else { this.#removeElementL10n(bottom); } - - this.#updateRowForButtons(item); - } - - #updateRowForButtons(item) { - if (item._buttons.size) { - item.setAttribute("has-buttons", "true"); - item.style.setProperty("--button-count", item._buttons.size); - for (let [name, button] of item._buttons) { - button.id = `${item.id}-button-${name}`; - } - - // When the row has buttons we set role=option on row-inner since the - // latter is the selectable logical row element when buttons are present. - // Since row-inner is not a child of the role=listbox element (the row - // container, this.#rows), screen readers will not automatically recognize - // it as a listbox option. To compensate, set role=presentation on the row - // so that screen readers ignore it. - item.setAttribute("role", "presentation"); - } else { - item.removeAttribute("has-buttons"); - item.style.removeProperty("--button-count"); - item.setAttribute("role", "option"); - } } /** @@ -1993,6 +1930,10 @@ export class UrlbarView { element, { updateInput = true, setAccessibleFocus = true } = {} ) { + if (element && !element.matches(SELECTABLE_ELEMENT_SELECTOR)) { + throw new Error("Element is not selectable"); + } + if (this.#selectedElement) { this.#selectedElement.toggleAttribute("selected", false); this.#selectedElement.removeAttribute("aria-selected"); @@ -2039,15 +1980,7 @@ export class UrlbarView { */ #getClosestSelectableElement(element) { let closest = element.closest(SELECTABLE_ELEMENT_SELECTOR); - if (!closest) { - let row = element.closest(".urlbarView-row"); - if (row && row.querySelector(UNSELECTABLE_ELEMENT_SELECTOR)) { - return null; - } else if (row && !row.querySelector(SELECTABLE_ELEMENT_SELECTOR)) { - closest = row; - } - } - return this.#isElementVisible(closest) ? closest : null; + return closest && this.#isElementVisible(closest) ? closest : null; } /** @@ -2083,15 +2016,7 @@ export class UrlbarView { * The last selectable element in the view. */ #getLastSelectableElement() { - let row = this.#rows.lastElementChild; - if (!row) { - return null; - } - let selectables = row.querySelectorAll(SELECTABLE_ELEMENT_SELECTOR); - let element = selectables.length - ? selectables[selectables.length - 1] - : row; - + let element = this.#rows.lastElementChild; if (element && !this.#isSelectableElement(element)) { element = this.#getPreviousSelectableElement(element); } @@ -2173,17 +2098,7 @@ export class UrlbarView { * item. */ #getSelectedRow() { - if (!this.isOpen || !this.#selectedElement) { - return null; - } - let selected = this.#selectedElement; - - if (!selected.classList.contains("urlbarView-row")) { - // selected may be an element in a result group, like RESULT_TYPE.TIP. - selected = selected.closest(".urlbarView-row"); - } - - return selected; + return this.#getRowFromElement(this.#selectedElement); } /** @@ -2193,15 +2108,7 @@ export class UrlbarView { * The row containing `element`, or `element` itself if it is a row. */ #getRowFromElement(element) { - if (!this.isOpen || !element) { - return null; - } - - if (!element.classList.contains("urlbarView-row")) { - element = element.closest(".urlbarView-row"); - } - - return element; + return element?.closest(".urlbarView-row"); } #setAccessibleFocus(item) { @@ -2221,6 +2128,18 @@ export class UrlbarView { * The DOM node for the result's tile. */ #setResultTitle(result, titleNode) { + if (result.payload.titleL10n) { + this.#setElementL10n(titleNode, result.payload.titleL10n); + return; + } + + // TODO: `text` is intended only for WebExtensions. We should remove it and + // the WebExtensions urlbar API since we're no longer using it. + if (result.payload.text) { + titleNode.textContent = result.payload.text; + return; + } + if (result.payload.providesSearchMode) { // Keyword offers are the only result that require a localized title. // We localize the title instead of using the action text as a title @@ -2338,9 +2257,7 @@ export class UrlbarView { if (result.type != lazy.UrlbarUtils.RESULT_TYPE.TIP) { return false; } - let tipButton = this.#rows.firstElementChild.querySelector( - ".urlbarView-tip-button" - ); + let tipButton = this.#rows.firstElementChild._buttons.get("0"); if (!tipButton) { throw new Error("Expected a tip button"); } diff --git a/browser/components/urlbar/docs/overview.rst b/browser/components/urlbar/docs/overview.rst index 427fa4b534bf..0ff1ee5ad797 100644 --- a/browser/components/urlbar/docs/overview.rst +++ b/browser/components/urlbar/docs/overview.rst @@ -406,9 +406,7 @@ The following RESULT_TYPEs are supported: // Payload: { icon, url, device, title } REMOTE_TAB: 6, // An actionable message to help the user with their query. - // textData and buttonTextData are objects containing an l10n id and args. - // If a tip is untranslated it's possible to provide text and buttonText. - // Payload: { icon, textData, buttonTextData, [buttonUrl], [helpUrl] } + // Payload: { buttons, helpL10n, helpUrl, icon, titleL10n, type } TIP: 7, // A type of result created at runtime, for example by an extension. // Payload: { dynamicType } diff --git a/browser/components/urlbar/tests/browser-tips/browser_glean_telemetry_engagement.js b/browser/components/urlbar/tests/browser-tips/browser_glean_telemetry_engagement.js index 3f8be30e1cf0..2b7d8f55e87e 100644 --- a/browser/components/urlbar/tests/browser-tips/browser_glean_telemetry_engagement.js +++ b/browser/components/urlbar/tests/browser-tips/browser_glean_telemetry_engagement.js @@ -43,7 +43,17 @@ add_task(async function selected_result_tip() { new UrlbarResult( UrlbarUtils.RESULT_TYPE.TIP, UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { type } + { + type, + helpUrl: "https://example.com/", + titleL10n: { id: "urlbar-search-tips-confirm" }, + buttons: [ + { + url: "https://example.com/", + l10n: { id: "urlbar-search-tips-confirm" }, + }, + ], + } ), ], priority: 1, diff --git a/browser/components/urlbar/tests/browser-tips/browser_interventions.js b/browser/components/urlbar/tests/browser-tips/browser_interventions.js index ec2fa76471fd..6add58746ac1 100644 --- a/browser/components/urlbar/tests/browser-tips/browser_interventions.js +++ b/browser/components/urlbar/tests/browser-tips/browser_interventions.js @@ -240,9 +240,14 @@ add_task(async function pickHelpButton() { UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, { type: UrlbarProviderInterventions.TIP_TYPE.CLEAR, - text: "This is a test tip.", - buttonText: "Done", + titleL10n: { id: "intervention-clear-data" }, + buttons: [ + { + l10n: { id: "intervention-clear-data-confirm" }, + }, + ], helpUrl, + helpL10n: { id: "urlbar-tip-help-icon" }, } ), ]; @@ -263,8 +268,12 @@ add_task(async function pickHelpButton() { UrlbarProviderInterventions.TIP_TYPE.CLEAR ); - let helpButton = element._elements.get("helpButton"); - Assert.ok(BrowserTestUtils.is_visible(helpButton)); + let helpButton = element._buttons.get("help"); + Assert.ok(helpButton, "Help button exists"); + Assert.ok( + BrowserTestUtils.is_visible(helpButton), + "Help button is visible" + ); EventUtils.synthesizeMouseAtCenter(helpButton, {}); BrowserTestUtils.loadURI(gBrowser.selectedBrowser, helpUrl); diff --git a/browser/components/urlbar/tests/browser-tips/browser_picks.js b/browser/components/urlbar/tests/browser-tips/browser_picks.js index b63c44f35e2a..51ebc7725b82 100644 --- a/browser/components/urlbar/tests/browser-tips/browser_picks.js +++ b/browser/components/urlbar/tests/browser-tips/browser_picks.js @@ -46,19 +46,7 @@ add_task(async function mouse_helpButton() { // Clicks inside a tip but not on any button. add_task(async function mouse_insideTipButNotOnButtons() { - let results = [ - new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - type: "test", - text: "This is a test tip.", - buttonText: "Done", - helpUrl: HELP_URL, - buttonUrl: TIP_URL, - } - ), - ]; + let results = [makeTipResult({ buttonUrl: TIP_URL, helpUrl: HELP_URL })]; let provider = new UrlbarTestUtils.TestProvider({ results, priority: 1 }); UrlbarProvidersManager.registerProvider(provider); @@ -79,7 +67,7 @@ add_task(async function mouse_insideTipButNotOnButtons() { ); Assert.equal( UrlbarTestUtils.getSelectedElement(window), - row._elements.get("tipButton"), + row._buttons.get("0"), "The main button element should be selected initially" ); EventUtils.synthesizeMouseAtCenter(row, {}); @@ -93,7 +81,7 @@ add_task(async function mouse_insideTipButNotOnButtons() { ); Assert.equal( UrlbarTestUtils.getSelectedElement(window), - row._elements.get("tipButton"), + row._buttons.get("0"), "The main button element should remain selected" ); @@ -128,19 +116,7 @@ async function doTest({ click, buttonUrl = undefined, helpUrl = undefined }) { // Add our test provider. let provider = new UrlbarTestUtils.TestProvider({ - results: [ - new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - type: "test", - text: "This is a test tip.", - buttonText: "Done", - buttonUrl, - helpUrl, - } - ), - ], + results: [makeTipResult({ buttonUrl, helpUrl })], priority: 1, }); UrlbarProvidersManager.registerProvider(provider); @@ -159,8 +135,8 @@ async function doTest({ click, buttonUrl = undefined, helpUrl = undefined }) { fireInputEvent: true, }); let row = await UrlbarTestUtils.waitForAutocompleteResultAt(window, 0); - let mainButton = row._elements.get("tipButton"); - let helpButton = row._elements.get("helpButton"); + let mainButton = row._buttons.get("0"); + let helpButton = row._buttons.get("help"); let target = helpUrl ? helpButton : mainButton; // If we're picking the tip with the keyboard, arrow down to select the proper @@ -213,3 +189,22 @@ async function doTest({ click, buttonUrl = undefined, helpUrl = undefined }) { BrowserTestUtils.removeTab(tab); } } + +function makeTipResult({ buttonUrl, helpUrl }) { + return new UrlbarResult( + UrlbarUtils.RESULT_TYPE.TIP, + UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, + { + type: "test", + titleL10n: { id: "urlbar-search-tips-confirm" }, + buttons: [ + { + url: buttonUrl, + l10n: { id: "urlbar-search-tips-confirm" }, + }, + ], + helpUrl, + helpL10n: { id: "urlbar-search-tips-confirm" }, + } + ); +} diff --git a/browser/components/urlbar/tests/browser-tips/browser_searchTips_interaction.js b/browser/components/urlbar/tests/browser-tips/browser_searchTips_interaction.js index 601747fdc9d4..7e4f0bbc6559 100644 --- a/browser/components/urlbar/tests/browser-tips/browser_searchTips_interaction.js +++ b/browser/components/urlbar/tests/browser-tips/browser_searchTips_interaction.js @@ -126,7 +126,7 @@ add_task(async function pickButton_onboard() { // Click the tip button. let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0); - let button = result.element.row._elements.get("tipButton"); + let button = result.element.row._buttons.get("0"); await UrlbarTestUtils.promisePopupClose(window, () => { EventUtils.synthesizeMouseAtCenter(button, {}); }); @@ -184,7 +184,7 @@ add_task(async function pickButton_redirect() { // Click the tip button. let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0); - let button = result.element.row._elements.get("tipButton"); + let button = result.element.row._buttons.get("0"); await UrlbarTestUtils.promisePopupClose(window, () => { EventUtils.synthesizeMouseAtCenter(button, {}); }); @@ -250,7 +250,7 @@ add_task(async function pickButton_persist() { await checkTip(window, UrlbarProviderSearchTips.TIP_TYPE.PERSIST, false); let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0); - let button = result.element.row._elements.get("tipButton"); + let button = result.element.row._buttons.get("0"); await UrlbarTestUtils.promisePopupClose(window, () => { EventUtils.synthesizeMouseAtCenter(button, {}); @@ -679,7 +679,7 @@ add_task(async function pickingTipDoesNotDisableOtherKinds() { // Click the tip button. let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0); - let button = result.element.row._elements.get("tipButton"); + let button = result.element.row._buttons.get("0"); await UrlbarTestUtils.promisePopupClose(window, () => { EventUtils.synthesizeMouseAtCenter(button, {}); }); diff --git a/browser/components/urlbar/tests/browser-tips/browser_selection.js b/browser/components/urlbar/tests/browser-tips/browser_selection.js index c25d21b2aa61..1002bb3fe4cc 100644 --- a/browser/components/urlbar/tests/browser-tips/browser_selection.js +++ b/browser/components/urlbar/tests/browser-tips/browser_selection.js @@ -15,18 +15,7 @@ add_task(async function tipIsSecondResult() { UrlbarUtils.RESULT_SOURCE.HISTORY, { url: "http://mozilla.org/a" } ), - new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - icon: "", - text: "This is a test intervention.", - buttonText: "Done", - type: "test", - helpUrl: HELP_URL, - buttonUrl: TIP_URL, - } - ), + makeTipResult({ buttonUrl: TIP_URL, helpUrl: HELP_URL }), ]; let provider = new UrlbarTestUtils.TestProvider({ results, priority: 1 }); @@ -59,7 +48,7 @@ add_task(async function tipIsSecondResult() { EventUtils.synthesizeKey("KEY_ArrowDown"); Assert.ok( UrlbarTestUtils.getSelectedElement(window).classList.contains( - "urlbarView-tip-button" + "urlbarView-button-0" ), "The selected element should be the tip button." ); @@ -122,20 +111,7 @@ add_task(async function tipIsSecondResult() { }); add_task(async function tipIsOnlyResult() { - let results = [ - new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - icon: "", - text: "This is a test intervention.", - buttonText: "Done", - type: "test", - helpUrl: - "https://support.mozilla.org/en-US/kb/delete-browsing-search-download-history-firefox", - } - ), - ]; + let results = [makeTipResult({ buttonUrl: TIP_URL, helpUrl: HELP_URL })]; let provider = new UrlbarTestUtils.TestProvider({ results, priority: 1 }); UrlbarProvidersManager.registerProvider(provider); @@ -160,7 +136,7 @@ add_task(async function tipIsOnlyResult() { EventUtils.synthesizeKey("KEY_ArrowDown"); Assert.ok( UrlbarTestUtils.getSelectedElement(window).classList.contains( - "urlbarView-tip-button" + "urlbarView-button-0" ), "The selected element should be the tip button." ); @@ -209,16 +185,7 @@ add_task(async function tipHasNoHelpButton() { UrlbarUtils.RESULT_SOURCE.HISTORY, { url: "http://mozilla.org/a" } ), - new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - icon: "", - text: "This is a test intervention.", - buttonText: "Done", - type: "test", - } - ), + makeTipResult({ buttonUrl: TIP_URL }), ]; let provider = new UrlbarTestUtils.TestProvider({ results, priority: 1 }); @@ -251,7 +218,7 @@ add_task(async function tipHasNoHelpButton() { EventUtils.synthesizeKey("KEY_ArrowDown"); Assert.ok( UrlbarTestUtils.getSelectedElement(window).classList.contains( - "urlbarView-tip-button" + "urlbarView-button-0" ), "The selected element should be the tip button." ); @@ -274,7 +241,7 @@ add_task(async function tipHasNoHelpButton() { EventUtils.synthesizeKey("KEY_ArrowUp"); Assert.ok( UrlbarTestUtils.getSelectedElement(window).classList.contains( - "urlbarView-tip-button" + "urlbarView-button-0" ), "The selected element should be the tip button." ); diff --git a/browser/components/urlbar/tests/browser-tips/head.js b/browser/components/urlbar/tests/browser-tips/head.js index 90bace795171..6fce8d367f67 100644 --- a/browser/components/urlbar/tests/browser-tips/head.js +++ b/browser/components/urlbar/tests/browser-tips/head.js @@ -325,7 +325,7 @@ async function doUpdateTest({ Assert.ok(title.test(actualTitle), "Title regexp"); } - let actualButton = element._elements.get("tipButton").textContent; + let actualButton = element._buttons.get("0").textContent; if (typeof button == "string") { Assert.equal(actualButton, button, "Button string"); } else { @@ -333,10 +333,7 @@ async function doUpdateTest({ Assert.ok(button.test(actualButton), "Button regexp"); } - Assert.ok( - BrowserTestUtils.is_visible(element._elements.get("helpButton")), - "Help button visible" - ); + Assert.ok(element._buttons.has("help"), "Tip has a help button"); // Pick the tip and wait for the action. let values = await Promise.all([awaitCallback(), pickTip()]); @@ -389,7 +386,7 @@ async function awaitTip(searchString, win = window) { */ async function pickTip() { let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 1); - let button = result.element.row._elements.get("tipButton"); + let button = result.element.row._buttons.get("0"); await UrlbarTestUtils.promisePopupClose(window, () => { EventUtils.synthesizeMouseAtCenter(button, {}); }); @@ -483,7 +480,7 @@ function checkIntervention({ Assert.ok(title.test(actualTitle), "Title regexp"); } - let actualButton = element._elements.get("tipButton").textContent; + let actualButton = element._buttons.get("0").textContent; if (typeof button == "string") { Assert.equal(actualButton, button, "Button string"); } else { @@ -491,7 +488,12 @@ function checkIntervention({ Assert.ok(button.test(actualButton), "Button regexp"); } - Assert.ok(BrowserTestUtils.is_visible(element._elements.get("helpButton"))); + let helpButton = element._buttons.get("help"); + Assert.ok(helpButton, "Help button exists"); + Assert.ok( + BrowserTestUtils.is_visible(helpButton), + "Help button is visible" + ); let values = await Promise.all([awaitCallback(), pickTip()]); Assert.ok(true, "Refresh dialog opened"); @@ -590,14 +592,12 @@ async function checkTip(win, expectedTip, closeView = true) { Assert.equal(result.heuristic, heuristic); Assert.equal(result.displayed.title, title); Assert.equal( - result.element.row._elements.get("tipButton").textContent, + result.element.row._buttons.get("0").textContent, expectedTip == UrlbarProviderSearchTips.TIP_TYPE.PERSIST ? `Got it` : `Okay, Got It` ); - Assert.ok( - BrowserTestUtils.is_hidden(result.element.row._elements.get("helpButton")) - ); + Assert.ok(!result.element.row._buttons.has("help")); const scalars = TelemetryTestUtils.getProcessScalars("parent", true, true); TelemetryTestUtils.assertKeyedScalar( @@ -617,6 +617,24 @@ async function checkTip(win, expectedTip, closeView = true) { } } +function makeTipResult({ buttonUrl, helpUrl = undefined }) { + return new UrlbarResult( + UrlbarUtils.RESULT_TYPE.TIP, + UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, + { + helpUrl, + type: "test", + titleL10n: { id: "urlbar-search-tips-confirm" }, + buttons: [ + { + url: buttonUrl, + l10n: { id: "urlbar-search-tips-confirm" }, + }, + ], + } + ); +} + /** * Search tips helper. Opens a foreground tab and asserts that a particular * search tip is shown or that no search tip is shown. diff --git a/browser/components/urlbar/tests/browser-updateResults/browser.ini b/browser/components/urlbar/tests/browser-updateResults/browser.ini index 7221e6ff8c55..3f27229fcdbc 100644 --- a/browser/components/urlbar/tests/browser-updateResults/browser.ini +++ b/browser/components/urlbar/tests/browser-updateResults/browser.ini @@ -7,7 +7,6 @@ support-files = head.js [browser_appendSpanCount.js] -[browser_reuse.js] [browser_suggestedIndex_5_search_5_url.js] [browser_suggestedIndex_5_search_10_url.js] [browser_suggestedIndex_5_url_5_search.js] diff --git a/browser/components/urlbar/tests/browser-updateResults/browser_reuse.js b/browser/components/urlbar/tests/browser-updateResults/browser_reuse.js deleted file mode 100644 index e3f256e527d5..000000000000 --- a/browser/components/urlbar/tests/browser-updateResults/browser_reuse.js +++ /dev/null @@ -1,235 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -// Tests row updating and reuse. - -"use strict"; - -let TEST_BASE_URL = "http://example.com/"; - -// 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 tipResult = new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - text: "This is a test tip.", - buttonText: "OK", - helpUrl: "http://example.com/", - type: "test", - } - ); - tipResult.suggestedIndex = 1; - let provider = new UrlbarTestUtils.TestProvider({ - results: [tipResult], - }); - provider.isActive = context => context.searchString == "testx"; - UrlbarProvidersManager.registerProvider(provider); - - // Search for "test". - await UrlbarTestUtils.promiseAutocompleteResultPopup({ - value: "test", - window, - }); - - // 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: TEST_BASE_URL + "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: TEST_BASE_URL + "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 tipResult = new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - text: "This is a test tip.", - buttonText: "OK", - helpUrl: "http://example.com/", - type: "test", - } - ); - tipResult.suggestedIndex = 1; - let provider = new UrlbarTestUtils.TestProvider({ - results: [tipResult], - }); - provider.isActive = context => - ["test", "testxx"].includes(context.searchString); - UrlbarProvidersManager.registerProvider(provider); - - // Search for "test". - await UrlbarTestUtils.promiseAutocompleteResultPopup({ - value: "test", - window, - }); - - // 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: TEST_BASE_URL + "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: TEST_BASE_URL + "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`); - } -} diff --git a/browser/components/urlbar/tests/browser/browser_bestMatch.js b/browser/components/urlbar/tests/browser/browser_bestMatch.js index 101a35b1b72b..35bcecf673ae 100644 --- a/browser/components/urlbar/tests/browser/browser_bestMatch.js +++ b/browser/components/urlbar/tests/browser/browser_bestMatch.js @@ -37,27 +37,6 @@ add_task(async function nonsponsoredHelpButton() { }); }); -// Tests a non-sponsored best match row with help and block buttons. -add_task(async function nonsponsoredHelpAndBlockButtons() { - await SpecialPowers.pushPrefEnv({ - set: [["browser.urlbar.bestMatch.blockingEnabled", true]], - }); - let result = makeBestMatchResult({ helpUrl: "https://example.com/help" }); - await withProvider(result, async () => { - await UrlbarTestUtils.promiseAutocompleteResultPopup({ - window, - value: "test", - }); - await checkBestMatchRow({ - result, - hasHelpButton: true, - hasBlockButton: true, - }); - await UrlbarTestUtils.promisePopupClose(window); - }); - await SpecialPowers.popPrefEnv(); -}); - // Tests a sponsored best match row. add_task(async function sponsored() { let result = makeBestMatchResult({ isSponsored: true }); @@ -87,31 +66,6 @@ add_task(async function sponsoredHelpButton() { }); }); -// Tests a sponsored best match row with help and block buttons. -add_task(async function sponsoredHelpAndBlockButtons() { - await SpecialPowers.pushPrefEnv({ - set: [["browser.urlbar.bestMatch.blockingEnabled", true]], - }); - let result = makeBestMatchResult({ - isSponsored: true, - helpUrl: "https://example.com/help", - }); - await withProvider(result, async () => { - await UrlbarTestUtils.promiseAutocompleteResultPopup({ - window, - value: "test", - }); - await checkBestMatchRow({ - result, - isSponsored: true, - hasHelpButton: true, - hasBlockButton: true, - }); - await UrlbarTestUtils.promisePopupClose(window); - }); - await SpecialPowers.popPrefEnv(); -}); - // Tests keyboard selection. add_task(async function keySelection() { let result = makeBestMatchResult({ @@ -121,79 +75,59 @@ add_task(async function keySelection() { await withProvider(result, async () => { // Ordered list of class names of the elements that should be selected. - let expectedClassNames = [ - "urlbarView-row-inner", - "urlbarView-button-block", - "urlbarView-button-help", - ]; + let expectedClassNames = ["urlbarView-row-inner", "urlbarView-button-help"]; - // Test with and without the block button. - for (let showBlockButton of [false, true]) { - UrlbarPrefs.set("bestMatch.blockingEnabled", showBlockButton); + await UrlbarTestUtils.promiseAutocompleteResultPopup({ + window, + value: "test", + }); + await checkBestMatchRow({ + result, + isSponsored: true, + hasHelpButton: true, + }); - // The block button is not immediately removed or added when - // `bestMatch.blockingEnabled` is toggled while the panel is open, so we - // need to do a new search each time we change it. - await UrlbarTestUtils.promiseAutocompleteResultPopup({ - window, - value: "test", - }); - await checkBestMatchRow({ - result, - isSponsored: true, - hasHelpButton: true, - hasBlockButton: showBlockButton, - }); + // Test with the tab key vs. arrow keys and in order vs. reverse order. + for (let useTabKey of [false, true]) { + for (let reverse of [false, true]) { + info("Doing key selection: " + JSON.stringify({ useTabKey, reverse })); - // Test with the tab key vs. arrow keys and in order vs. reverse order. - for (let useTabKey of [false, true]) { - for (let reverse of [false, true]) { - info( - "Doing key selection: " + - JSON.stringify({ showBlockButton, useTabKey, reverse }) - ); + let classNames = [...expectedClassNames]; + if (reverse) { + classNames.reverse(); + } - let classNames = [...expectedClassNames]; - if (!showBlockButton) { - classNames.splice(classNames.indexOf("urlbarView-button-block"), 1); - } - if (reverse) { - classNames.reverse(); + let sendKey = () => { + if (useTabKey) { + EventUtils.synthesizeKey("KEY_Tab", { shiftKey: reverse }); + } else if (reverse) { + EventUtils.synthesizeKey("KEY_ArrowUp"); + } else { + EventUtils.synthesizeKey("KEY_ArrowDown"); } + }; - let sendKey = () => { - if (useTabKey) { - EventUtils.synthesizeKey("KEY_Tab", { shiftKey: reverse }); - } else if (reverse) { - EventUtils.synthesizeKey("KEY_ArrowUp"); - } else { - EventUtils.synthesizeKey("KEY_ArrowDown"); - } - }; - - // Move selection through each expected element. - for (let className of classNames) { - info("Expecting selection: " + className); - sendKey(); - Assert.ok(gURLBar.view.isOpen, "View remains open"); - let { selectedElement } = gURLBar.view; - Assert.ok(selectedElement, "Selected element exists"); - Assert.ok( - selectedElement.classList.contains(className), - "Expected element is selected" - ); - } + // Move selection through each expected element. + for (let className of classNames) { + info("Expecting selection: " + className); sendKey(); + Assert.ok(gURLBar.view.isOpen, "View remains open"); + let { selectedElement } = gURLBar.view; + Assert.ok(selectedElement, "Selected element exists"); Assert.ok( - gURLBar.view.isOpen, - "View remains open after keying through best match row" + selectedElement.classList.contains(className), + "Expected element is selected" ); } + sendKey(); + Assert.ok( + gURLBar.view.isOpen, + "View remains open after keying through best match row" + ); } - - await UrlbarTestUtils.promisePopupClose(window); - UrlbarPrefs.clear("bestMatch.blockingEnabled"); } + + await UrlbarTestUtils.promisePopupClose(window); }); }); @@ -201,7 +135,6 @@ async function checkBestMatchRow({ result, isSponsored = false, hasHelpButton = false, - hasBlockButton = false, }) { Assert.equal( UrlbarTestUtils.getResultCount(window), @@ -252,13 +185,6 @@ async function checkBestMatchRow({ ); } - let blockButton = row._buttons.get("block"); - if (hasBlockButton) { - Assert.ok(blockButton, "Row has a block button"); - } else { - Assert.ok(!blockButton, "Row does not have a block button"); - } - let helpButton = row._buttons.get("help"); Assert.equal( !!result.payload.helpUrl, diff --git a/browser/components/urlbar/tests/browser/browser_helpUrl.js b/browser/components/urlbar/tests/browser/browser_helpUrl.js index 589b0dc2f496..db6826e131c7 100644 --- a/browser/components/urlbar/tests/browser/browser_helpUrl.js +++ b/browser/components/urlbar/tests/browser/browser_helpUrl.js @@ -21,11 +21,11 @@ add_setup(async function() { }); }); -// Sets `helpL10nId` on the result payload and makes sure the help button ends +// Sets `helpL10n` on the result payload and makes sure the help button ends // up with a corresponding l10n attribute. -add_task(async function title_helpL10nId() { - let helpL10nId = "urlbar-tip-help-icon"; - let provider = registerTestProvider(1, { helpL10nId }); +add_task(async function title_helpL10n() { + let helpL10n = { id: "urlbar-tip-help-icon" }; + let provider = registerTestProvider(1, { helpL10n }); await UrlbarTestUtils.promiseAutocompleteResultPopup({ value: "example", window, @@ -40,7 +40,7 @@ add_task(async function title_helpL10nId() { let l10nAttrs = document.l10n.getAttributes(helpButton); Assert.deepEqual( l10nAttrs, - { id: helpL10nId, args: null }, + { id: helpL10n.id, args: null }, "The l10n ID attribute was correctly set" ); diff --git a/browser/components/urlbar/tests/browser/browser_heuristicNotAddedFirst.js b/browser/components/urlbar/tests/browser/browser_heuristicNotAddedFirst.js index a51921fc9200..c418bf2e8380 100644 --- a/browser/components/urlbar/tests/browser/browser_heuristicNotAddedFirst.js +++ b/browser/components/urlbar/tests/browser/browser_heuristicNotAddedFirst.js @@ -32,16 +32,7 @@ add_task(async function slowHeuristicSelected() { // Second, add another provider that adds a non-heuristic result immediately // with suggestedIndex = 1. - let nonHeuristicResult = new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - text: "This is a test tip.", - buttonText: "Done", - type: "test", - helpUrl: "http://example.com/", - } - ); + let nonHeuristicResult = makeTipResult(); nonHeuristicResult.suggestedIndex = 1; let nonHeuristicProvider = new UrlbarTestUtils.TestProvider({ results: [nonHeuristicResult], @@ -103,16 +94,7 @@ add_task(async function oneOffRemainsSelected() { // Second, add another provider that adds a non-heuristic result immediately // with suggestedIndex = 1. - let nonHeuristicResult = new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - text: "This is a test tip.", - buttonText: "Done", - type: "test", - helpUrl: "http://example.com/", - } - ); + let nonHeuristicResult = makeTipResult(); nonHeuristicResult.suggestedIndex = 1; let nonHeuristicProvider = new UrlbarTestUtils.TestProvider({ results: [nonHeuristicResult], @@ -161,3 +143,21 @@ add_task(async function oneOffRemainsSelected() { UrlbarProvidersManager.unregisterProvider(nonHeuristicProvider); await BrowserTestUtils.closeWindow(win); }); + +function makeTipResult() { + return new UrlbarResult( + UrlbarUtils.RESULT_TYPE.TIP, + UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, + { + helpUrl: "http://example.com/", + type: "test", + titleL10n: { id: "urlbar-search-tips-confirm" }, + buttons: [ + { + url: "http://example.com/", + l10n: { id: "urlbar-search-tips-confirm" }, + }, + ], + } + ); +} diff --git a/browser/components/urlbar/tests/browser/browser_resultSpan.js b/browser/components/urlbar/tests/browser/browser_resultSpan.js index 8f6a143a848e..fa779734272b 100644 --- a/browser/components/urlbar/tests/browser/browser_resultSpan.js +++ b/browser/components/urlbar/tests/browser/browser_resultSpan.js @@ -12,16 +12,7 @@ const TEST_RESULTS = [ UrlbarUtils.RESULT_SOURCE.HISTORY, { url: "http://mozilla.org/1" } ), - new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - text: "This is a test tip.", - buttonText: "Done", - type: "test", - helpUrl: "about:about", - } - ), + makeTipResult(), ]; const MAX_RESULTS = UrlbarPrefs.get("maxRichResults"); @@ -73,18 +64,7 @@ add_task(async function oneTip() { add_task(async function threeTips() { let results = Array.from(TEST_RESULTS); for (let i = 1; i < 3; i++) { - results.push( - new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - text: "This is a test tip.", - buttonText: "Done", - type: "test", - helpUrl: `about:about#${i}`, - } - ) - ); + results.push(makeTipResult()); } for (let i = 2; i < 15; i++) { results.push( @@ -160,18 +140,7 @@ add_task(async function oneTip_nonRestricting() { add_task(async function threeTips_nonRestricting() { let results = Array.from(TEST_RESULTS); for (let i = 1; i < 3; i++) { - results.push( - new UrlbarResult( - UrlbarUtils.RESULT_TYPE.TIP, - UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, - { - text: "This is a test tip.", - buttonText: "Done", - type: "test", - helpUrl: `about:about#${i}`, - } - ) - ); + results.push(makeTipResult()); } for (let i = 2; i < 15; i++) { results.push( @@ -265,3 +234,21 @@ function collectExpectedProperties(actualObj, expectedObj) { } return newActualObj; } + +function makeTipResult() { + return new UrlbarResult( + UrlbarUtils.RESULT_TYPE.TIP, + UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, + { + helpUrl: "http://example.com/", + type: "test", + titleL10n: { id: "urlbar-search-tips-confirm" }, + buttons: [ + { + url: "http://example.com/", + l10n: { id: "urlbar-search-tips-confirm" }, + }, + ], + } + ); +} diff --git a/browser/components/urlbar/tests/browser/browser_result_onSelection.js b/browser/components/urlbar/tests/browser/browser_result_onSelection.js index 329091ccf897..9409ab83b4a7 100644 --- a/browser/components/urlbar/tests/browser/browser_result_onSelection.js +++ b/browser/components/urlbar/tests/browser/browser_result_onSelection.js @@ -19,10 +19,15 @@ add_task(async function test() { UrlbarUtils.RESULT_TYPE.TIP, UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, { - text: "This is a test tip.", - buttonText: "Done", + helpUrl: "http://example.com/", type: "test", - helpUrl: "about:about", + titleL10n: { id: "urlbar-search-tips-confirm" }, + buttons: [ + { + url: "http://example.com/", + l10n: { id: "urlbar-search-tips-confirm" }, + }, + ], } ), ]; diff --git a/browser/components/urlbar/tests/browser/browser_suppressFocusBorder.js b/browser/components/urlbar/tests/browser/browser_suppressFocusBorder.js index 6456a06e6f4e..dfe7af971241 100644 --- a/browser/components/urlbar/tests/browser/browser_suppressFocusBorder.js +++ b/browser/components/urlbar/tests/browser/browser_suppressFocusBorder.js @@ -213,7 +213,7 @@ add_task(async function searchTip() { info("Click the tip button."); const result = await UrlbarTestUtils.getDetailsOfResultAt(win, 0); - const button = result.element.row._elements.get("tipButton"); + const button = result.element.row._buttons.get("0"); await UrlbarTestUtils.promisePopupClose(win, () => { EventUtils.synthesizeMouseAtCenter(button, {}, win); }); diff --git a/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry_engagement.js b/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry_engagement.js index d60ca2577f1e..7ab3057b729e 100644 --- a/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry_engagement.js +++ b/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry_engagement.js @@ -1267,10 +1267,15 @@ let tipMatches = [ UrlbarUtils.RESULT_TYPE.TIP, UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, { - text: "This is a test intervention.", - buttonText: "Done", + helpUrl: "http://example.com/", type: "test", - helpUrl: "about:about", + titleL10n: { id: "urlbar-search-tips-confirm" }, + buttons: [ + { + url: "http://example.com/", + l10n: { id: "urlbar-search-tips-confirm" }, + }, + ], } ), new UrlbarResult( diff --git a/browser/components/urlbar/tests/browser/browser_urlbar_telemetry_tip.js b/browser/components/urlbar/tests/browser/browser_urlbar_telemetry_tip.js index 4efcc09ef618..c99fa7d7aab3 100644 --- a/browser/components/urlbar/tests/browser/browser_urlbar_telemetry_tip.js +++ b/browser/components/urlbar/tests/browser/browser_urlbar_telemetry_tip.js @@ -58,10 +58,15 @@ add_task(async function test() { UrlbarUtils.RESULT_TYPE.TIP, UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, { - icon: "", - text: "This is a test tip.", - buttonText: "OK", + helpUrl: "https://example.com/", type: "test", + titleL10n: { id: "urlbar-search-tips-confirm" }, + buttons: [ + { + url: "https://example.com/", + l10n: { id: "urlbar-search-tips-confirm" }, + }, + ], } ), { heuristic: true } diff --git a/browser/components/urlbar/tests/browser/head-glean.js b/browser/components/urlbar/tests/browser/head-glean.js index c17ed8670f49..bd46ba1be46b 100644 --- a/browser/components/urlbar/tests/browser/head-glean.js +++ b/browser/components/urlbar/tests/browser/head-glean.js @@ -108,7 +108,7 @@ async function doClick() { async function doClickSubButton(selector) { const selected = UrlbarTestUtils.getSelectedElement(window); - const button = selected.querySelector(selector); + const button = selected.closest(".urlbarView-row").querySelector(selector); EventUtils.synthesizeMouseAtCenter(button, {}); } diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest.js index 08d4410a77a4..c17d3d0ea118 100644 --- a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest.js +++ b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest.js @@ -92,7 +92,9 @@ const EXPECTED_SPONSORED_RESULT = { sponsoredIabCategory: "22 - Shopping", isSponsored: true, helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: "http://test.com/q=frabbits", source: "remote-settings", }, @@ -115,7 +117,9 @@ const EXPECTED_NONSPONSORED_RESULT = { sponsoredIabCategory: "5 - Education", isSponsored: false, helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: "http://test.com/?q=nonsponsored", source: "remote-settings", }, @@ -138,7 +142,9 @@ const EXPECTED_HTTP_RESULT = { sponsoredIabCategory: "22 - Shopping", isSponsored: true, helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: "http://" + PREFIX_SUGGESTIONS_STRIPPED_URL, source: "remote-settings", }, @@ -161,7 +167,9 @@ const EXPECTED_HTTPS_RESULT = { sponsoredIabCategory: "22 - Shopping", isSponsored: true, helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: PREFIX_SUGGESTIONS_STRIPPED_URL, source: "remote-settings", }, @@ -937,7 +945,9 @@ add_task(async function dedupeAgainstURL_timestamps() { sponsoredIabCategory: "22 - Shopping", isSponsored: true, helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, source: "remote-settings", }, }; diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_bestMatch.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_bestMatch.js index 5b5af82fba8d..e43c24aea8df 100644 --- a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_bestMatch.js +++ b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_bestMatch.js @@ -63,7 +63,9 @@ const EXPECTED_BEST_MATCH_RESULT = { sponsoredBlockId: 1, sponsoredAdvertiser: "TestAdvertiser", helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: "http://example.com", source: "remote-settings", }, @@ -85,7 +87,9 @@ const EXPECTED_NON_BEST_MATCH_RESULT = { sponsoredBlockId: 1, sponsoredAdvertiser: "TestAdvertiser", helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: "http://example.com", source: "remote-settings", }, @@ -107,7 +111,9 @@ const EXPECTED_BEST_MATCH_POSITION_RESULT = { sponsoredBlockId: 2, sponsoredAdvertiser: "TestAdvertiser", helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: "http://example.com/best-match-position", source: "remote-settings", }, diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_impressionCaps.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_impressionCaps.js index e78ec666151f..642ae708ee7c 100644 --- a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_impressionCaps.js +++ b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_impressionCaps.js @@ -55,7 +55,9 @@ const EXPECTED_SPONSORED_RESULT = { sponsoredAdvertiser: "TestAdvertiser", sponsoredIabCategory: "22 - Shopping", helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, source: "remote-settings", }, }; @@ -78,7 +80,9 @@ const EXPECTED_NONSPONSORED_RESULT = { sponsoredAdvertiser: "TestAdvertiser", sponsoredIabCategory: "5 - Education", helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, source: "remote-settings", }, }; diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_merino.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_merino.js index aebe73b71de8..93448a58f6a7 100644 --- a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_merino.js +++ b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_merino.js @@ -41,7 +41,9 @@ const EXPECTED_REMOTE_SETTINGS_RESULT = { sponsoredAdvertiser: "TestAdvertiser", isSponsored: true, helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: "http://test.com/q=frabbits", source: "remote-settings", }, @@ -63,7 +65,9 @@ const EXPECTED_MERINO_RESULT = { sponsoredAdvertiser: "advertiser", isSponsored: true, helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: "url", requestId: "request_id", source: "merino", @@ -457,7 +461,9 @@ add_task(async function multipleMerinoSuggestions() { sponsoredAdvertiser: "multipleMerinoSuggestions 1 advertiser", isSponsored: true, helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: "multipleMerinoSuggestions 1 url", requestId: "request_id", source: "merino", @@ -671,7 +677,9 @@ add_task(async function topPick() { sponsoredAdvertiser: "multipleMerinoSuggestions 2 advertiser", isSponsored: true, helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: "multipleMerinoSuggestions 2 url", requestId: "request_id", source: "merino", diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_nonUniqueKeywords.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_nonUniqueKeywords.js index e8d38d318202..ccf3fa854dc7 100644 --- a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_nonUniqueKeywords.js +++ b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_nonUniqueKeywords.js @@ -190,7 +190,9 @@ add_task(async function() { sponsoredIabCategory: qsResult.iab_category, icon: null, helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, source: "remote-settings", }, }); diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_positionInSuggestions.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_positionInSuggestions.js index 69da696503cf..60c701a29eb7 100644 --- a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_positionInSuggestions.js +++ b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_positionInSuggestions.js @@ -157,7 +157,9 @@ function createExpectedQuickSuggestResult(suggest) { sponsoredIabCategory: suggest.iab_category, isSponsored: suggest.iab_category !== "5 - Education", helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, + isBlockable: false, + blockL10n: { id: "firefox-suggest-urlbar-block" }, displayUrl: suggest.url, source: "remote-settings", }, diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js b/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js index ed5fd7d181e8..5e403e1dd4aa 100644 --- a/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js +++ b/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js @@ -446,7 +446,7 @@ function makeExpectedResult(temperatureUnit) { url: MERINO_SUGGESTION.url, icon: "chrome://global/skin/icons/highlights.svg", helpUrl: QuickSuggest.HELP_URL, - helpL10nId: "firefox-suggest-urlbar-learn-more", + helpL10n: { id: "firefox-suggest-urlbar-learn-more" }, requestId: MerinoTestUtils.server.response.body.request_id, source: "merino", }, diff --git a/browser/themes/shared/urlbar-dynamic-results.css b/browser/themes/shared/urlbar-dynamic-results.css index 072668b72f56..db18fb759389 100644 --- a/browser/themes/shared/urlbar-dynamic-results.css +++ b/browser/themes/shared/urlbar-dynamic-results.css @@ -9,7 +9,6 @@ .urlbarView-row[dynamicType=onboardTabToSearch] > .urlbarView-row-inner { min-height: 64px !important; /* Overriding :root:not([uidensity=compact]) .urlbarView-row-inner { min-height } in urlbarView.inc.css */ align-items: center; - width: 100%; } .urlbarView-row[dynamicType=onboardTabToSearch] > .urlbarView-row-inner > .urlbarView-no-wrap { @@ -152,8 +151,7 @@ display: flex; flex-grow: 1; gap: 0.9em 1.8em; - /** parent width - help button width - gap to help button */ - max-width: calc(100% - 28px - 1.8em); + max-width: 100%; } #urlbar[searchmodesource=actions] .urlbarView-row[dynamicType=quickactions] .urlbarView-dynamic-quickactions-buttons { diff --git a/browser/themes/shared/urlbarView.css b/browser/themes/shared/urlbarView.css index 5ddbc5dc0af8..ad9065af9d9e 100644 --- a/browser/themes/shared/urlbarView.css +++ b/browser/themes/shared/urlbarView.css @@ -77,6 +77,8 @@ } .urlbarView-row { + display: flex; + align-items: center; fill: currentColor; fill-opacity: var(--urlbar-icon-fill-opacity); padding-block: 2px; @@ -91,7 +93,8 @@ } .urlbarView-row-inner { - display: flex; + display: inline-flex; + flex: 1 1; flex-wrap: nowrap; overflow: hidden; border-radius: var(--toolbarbutton-border-radius); @@ -103,33 +106,15 @@ min-height: 20px; /* row min-height 32px - (2 * padding-block 6px) */ } -.urlbarView-row[has-buttons] > .urlbarView-row-inner { - display: inline-flex; - vertical-align: middle; - /* For rows with buttons, row-inner is the main selectable part of the row, - and it takes up the entire row width except for the buttons. - 28px = 24px button width + 4px margin-inline between buttons - 12px = row-inner padding - 2px = horizontally align last button with the one-off settings button */ - width: calc(100% - (var(--button-count, 0) * 28px) - 12px - 2px); -} - -.urlbarView-row[type=tip] { - padding-block-start: 18px; - /* Compensating for the 16px bottom margin on the tip elements. */ - padding-block-end: calc(18px - 16px); - padding-inline-end: var(--urlbarView-icon-margin-end); -} - .urlbarView-row-inner, .urlbarView-no-wrap { - display: flex; flex-wrap: nowrap; align-items: center; justify-content: start; } .urlbarView-no-wrap { + display: inline-flex; max-width: 100%; flex-grow: 0; flex-shrink: 0; @@ -195,13 +180,6 @@ } } -/* We should always wrap tip results at narrow widths regardless of screen - height. Unlike regular results, unwrapped tips are taller than wrapped - tips. */ -.urlbarView-results[wrap] > .urlbarView-row[type=tip] > .urlbarView-row-inner { - flex-wrap: wrap; -} - .urlbarView-row:not([type=tip], [type=dynamic]) > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-title[overflow], .urlbarView-row[type=bestmatch] > .urlbarView-row-inner > .urlbarView-row-body > .urlbarView-row-body-top > .urlbarView-row-body-top-no-wrap > .urlbarView-title[overflow], .urlbarView-tags[overflow], @@ -228,13 +206,13 @@ text-align: right; } -.urlbarView-row:not([type=tip], [type=dynamic], [has-buttons]):hover > .urlbarView-row-inner, -.urlbarView-row[has-buttons] > .urlbarView-row-inner:not([selected]):hover { +.urlbarView-row[selectable]:hover > .urlbarView-row-inner, +.urlbarView-row-inner[selectable]:hover { background-color: var(--autocomplete-popup-hover-background); } -.urlbarView-row:not([type=tip], [type=dynamic])[selected] > .urlbarView-row-inner, -.urlbarView-row-inner[selected] { +.urlbarView-row[selectable][selected] > .urlbarView-row-inner, +.urlbarView-row-inner[selectable][selected] { background-color: var(--autocomplete-popup-highlight-background); color: var(--autocomplete-popup-highlight-color); } @@ -253,7 +231,7 @@ } -.urlbarView-row[type=tip] > .urlbarView-row-inner > .urlbarView-favicon { +.urlbarView-row[type=tip] > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-favicon { width: 24px; height: 24px; margin-inline-end: 12px; @@ -343,45 +321,14 @@ background-image: url("chrome://global/skin/icons/more.svg"); } -/* Tip rows */ - -.urlbarView-row[type=tip]:not(:last-child) { - border-bottom: 1px solid var(--panel-separator-color); - margin-bottom: 4px; -} - -.urlbarView-row[type=tip]:not(:first-child) { - border-top: 1px solid var(--panel-separator-color); - margin-top: 4px; -} - -.urlbarView-row[type=tip] > .urlbarView-row-inner { - display: flex; - align-items: center; - min-height: 32px; - width: 100%; -} - -/* For tips, give the title some bottom margin so that when the window is narrow - and the buttons wrap below it, there's space between it and the buttons. We - then need to give the same bottom margin to the other tip elements so that - they all remain vertically aligned. */ -.urlbarView-row[type=tip] > .urlbarView-row-inner > .urlbarView-favicon, -.urlbarView-row[type=tip] > .urlbarView-row-inner > .urlbarView-title, -.urlbarView-row[type=tip] > .urlbarView-row-inner > .urlbarView-button-help, -.urlbarView-tip-button { - margin-block-end: 16px; -} - -.urlbarView-tip-button, -.urlbarView-row:is([type=tip], [dynamicType="quickactions"]) > .urlbarView-row-inner > .urlbarView-button-help { +.urlbarView-button:not(:empty) { min-height: 16px; padding: 7px; } /* The tip button is a Photon default button when unfocused and a primary button in all other states. */ -.urlbarView-tip-button { +.urlbarView-button:not(:empty) { border-radius: var(--toolbarbutton-border-radius); font-size: 0.93em; background-color: var(--button-bgcolor); @@ -394,57 +341,79 @@ margin-inline-end: 4px; } -.urlbarView-tip-button:hover { +.urlbarView-button:not(:empty):hover { background-color: var(--button-hover-bgcolor); } -.urlbarView-tip-button:hover:active { +.urlbarView-button:not(:empty):hover:active { background-color: var(--button-active-bgcolor); } -.urlbarView-tip-button[selected] { +.urlbarView-button:not(:empty)[selected] { color: var(--button-primary-color); background-color: var(--button-primary-bgcolor); outline: var(--focus-outline); outline-offset: 2px; } -.urlbarView-tip-button[selected]:hover { +.urlbarView-button:not(:empty)[selected]:hover { background-color: var(--button-primary-hover-bgcolor); } -.urlbarView-tip-button[selected]:hover:active { +.urlbarView-button:not(:empty)[selected]:hover:active { background-color: var(--button-primary-active-bgcolor); } -.urlbarView-tip-button-spacer { - flex-basis: 48px; - flex-grow: 1; - flex-shrink: 1; +.urlbarView-button:not(:empty) + .urlbarView-button:empty { + /* Add space between a labeled button followed by an unlabeled button. */ + margin-inline-start: 1.8em; } -.urlbarView-row:is([type=tip], [dynamicType="quickactions"]) > .urlbarView-row-inner > .urlbarView-button-help { - min-width: 16px; - min-height: 16px; - margin-block-start: 2px; - margin-inline: 1.8em 0; +.urlbarView-button:not(:empty):last-child { + /* Add space between a labeled button and the trailing edge of the panel. */ + margin-inline-end: 1.8em; } -.urlbarView-row:is([dynamicType="quickactions"]) > .urlbarView-row-inner > .urlbarView-button-help { +/* When the last button is unlabeled, vertically align it with the settings icon + in the search shortcuts. */ +.urlbarView-button:empty:last-child { + margin-inline-end: 2px; +} + +/* Tip rows */ + +.urlbarView-row[type=tip] { + padding-block: 18px; +} + +.urlbarView-row[type=tip]:not(:last-child) { + border-bottom: 1px solid var(--panel-separator-color); + margin-bottom: 4px; +} + +.urlbarView-row[type=tip]:not(:first-child) { + border-top: 1px solid var(--panel-separator-color); + margin-top: 4px; +} + +.urlbarView-row[type=tip] > .urlbarView-row-inner { + min-height: 32px; + + /* Add space between the tip title (and the rest of row-inner) and its + button. */ + margin-inline-end: 1.8em; +} + +.urlbarView-row[type=tip] > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-title-separator, +.urlbarView-row[type=tip] > .urlbarView-row-inner > .urlbarView-url { display: none; } -.urlbarView-row:is([type=tip], [dynamicType="quickactions"]) > .urlbarView-row-inner > .urlbarView-button-help[selected] { - outline-offset: var(--focus-outline-offset); +.urlbarView-row[type=tip] > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-title { + white-space: normal; } -.urlbarView-row:is([type=tip], [dynamicType="quickactions"]) > .urlbarView-row-inner > .urlbarView-button-help:hover { - background-color: var(--button-hover-bgcolor); -} - -.urlbarView-row:is([type=tip], [dynamicType="quickactions"]) > .urlbarView-row-inner > .urlbarView-button-help:hover:active { - background-color: var(--button-active-bgcolor); -} +/* Row label (a.k.a. group label) */ .urlbarView-row[label]::before { content: attr(label); @@ -484,17 +453,6 @@ line-height: 1.4; } -.urlbarView-row[type=tip] > .urlbarView-row-inner > .urlbarView-title { - white-space: normal; - /* Give the tip title appropriate flex so that when the window is narrow, the - tip buttons wrap around under it. We want the icon and title to remain on - one line. So make the title's flex-basis the width of the parent minus the - width of the icon. */ - flex-basis: calc(100% - /* .urlbarView-row-inner padding-inline-start */ 6px - /* .urlbarView-favicon width */ 24px - /* .urlbarView-favicon margin-inline-end */ 12px); - flex-grow: 1; - flex-shrink: 1; -} - /* Tail suggestions */ .urlbarView-tail-prefix { display: none;