From a609266a088637d27b259d3e07153ef1a6bf5027 Mon Sep 17 00:00:00 2001 From: Harry Twyford Date: Wed, 25 Sep 2019 17:37:09 +0000 Subject: [PATCH] Bug 1578436 - Handle enter keypresses and mouse clicks on tip buttons. r=adw Differential Revision: https://phabricator.services.mozilla.com/D46067 --HG-- rename : browser/components/urlbar/tests/browser/browser_tip_keyboard_selection.js => browser/components/urlbar/tests/browser/browser_tip_selection.js extra : moz-landing-system : lando --- .../components/urlbar/UrlbarController.jsm | 25 ++-- browser/components/urlbar/UrlbarInput.jsm | 45 ++++-- browser/components/urlbar/UrlbarUtils.jsm | 5 + browser/components/urlbar/UrlbarView.jsm | 61 +++++++- .../urlbar/tests/UrlbarTestUtils.jsm | 2 +- .../urlbar/tests/browser/browser.ini | 2 +- ..._selection.js => browser_tip_selection.js} | 59 +++++++- .../browser/browser_urlbar_event_telemetry.js | 137 ++++++++++++++++++ .../themes/shared/urlbar-autocomplete.inc.css | 5 +- toolkit/components/telemetry/Events.yaml | 2 +- 10 files changed, 309 insertions(+), 34 deletions(-) rename browser/components/urlbar/tests/browser/{browser_tip_keyboard_selection.js => browser_tip_selection.js} (76%) diff --git a/browser/components/urlbar/UrlbarController.jsm b/browser/components/urlbar/UrlbarController.jsm index 6cedbfe07c8b..b7459403f3e2 100644 --- a/browser/components/urlbar/UrlbarController.jsm +++ b/browser/components/urlbar/UrlbarController.jsm @@ -782,25 +782,29 @@ class TelemetryEvent { } /** - * Extracts a type from a result, to be used in the telemetry event. - * @param {UrlbarResult} result The result to analyze. + * Extracts a type from an element, to be used in the telemetry event. + * @param {Element} element The element to analyze. * @returns {string} a string type for the telemetry event. */ - typeFromResult(result) { - if (result) { - switch (result.type) { + typeFromElement(element) { + if (!element) { + return "none"; + } + let row = element.closest(".urlbarView-row"); + if (row.result) { + switch (row.result.type) { case UrlbarUtils.RESULT_TYPE.TAB_SWITCH: return "switchtab"; case UrlbarUtils.RESULT_TYPE.SEARCH: - return result.payload.suggestion ? "searchsuggestion" : "search"; + return row.result.payload.suggestion ? "searchsuggestion" : "search"; case UrlbarUtils.RESULT_TYPE.URL: - if (result.autofill) { + if (row.result.autofill) { return "autofill"; } - if (result.heuristic) { + if (row.result.heuristic) { return "visit"; } - return result.source == UrlbarUtils.RESULT_SOURCE.BOOKMARKS + return row.result.source == UrlbarUtils.RESULT_SOURCE.BOOKMARKS ? "bookmark" : "history"; case UrlbarUtils.RESULT_TYPE.KEYWORD: @@ -810,6 +814,9 @@ class TelemetryEvent { case UrlbarUtils.RESULT_TYPE.REMOTE_TAB: return "remotetab"; case UrlbarUtils.RESULT_TYPE.TIP: + if (element.classList.contains("urlbarView-tip-help")) { + return "tiphelp"; + } return "tip"; } } diff --git a/browser/components/urlbar/UrlbarInput.jsm b/browser/components/urlbar/UrlbarInput.jsm index dedb0f1deec1..61b27d50f87b 100644 --- a/browser/components/urlbar/UrlbarInput.jsm +++ b/browser/components/urlbar/UrlbarInput.jsm @@ -407,16 +407,18 @@ class UrlbarInput { } } - // Use the selected result if we have one; this is usually the case + // Use the selected element if we have one; this is usually the case // when the view is open. - let result = this.view.selectedResult; - if (!selectedOneOff && result) { - this.pickResult(result, event); + let element = this.view.selectedElement; + if (!selectedOneOff && element) { + this.pickElement(element, event); return; } + let result = this.view.getResultFromElement(element); + let url; - let selType = this.controller.engagementEvent.typeFromResult(result); + let selType = this.controller.engagementEvent.typeFromElement(element); let numChars = this.value.length; if (selectedOneOff) { selType = "oneoff"; @@ -489,13 +491,17 @@ class UrlbarInput { } /** - * Called by the view when a result is picked. + * Called by the view when an element is picked. * - * @param {UrlbarResult} result The result that was picked. - * @param {Event} event The event that picked the result. + * @param {Element} element The element that was picked. + * @param {Event} event The event that picked the element. */ - pickResult(result, event) { + pickElement(element, event) { let originalUntrimmedValue = this.untrimmedValue; + let result = this.view.getResultFromElement(element); + if (!result) { + return; + } let isCanonized = this.setValueFromResult(result, event); let where = this._whereToOpen(event); let openParams = { @@ -646,6 +652,25 @@ class UrlbarInput { this._recordSearch(engine, event, actionDetails); break; } + case UrlbarUtils.RESULT_TYPE.TIP: { + if (element.classList.contains("urlbarView-tip-help")) { + url = result.payload.helpUrl; + } + + if (!url) { + this.handleRevert(); + this.controller.engagementEvent.record(event, { + numChars: this._lastSearchString.length, + selIndex, + selType: "tip", + }); + + // TODO: Call out to UrlbarProvider.pickElement as part of bug 1578584. + return; + } + + break; + } case UrlbarUtils.RESULT_TYPE.OMNIBOX: { this.controller.engagementEvent.record(event, { numChars: this._lastSearchString.length, @@ -685,7 +710,7 @@ class UrlbarInput { this.controller.engagementEvent.record(event, { numChars: this._lastSearchString.length, selIndex, - selType: this.controller.engagementEvent.typeFromResult(result), + selType: this.controller.engagementEvent.typeFromElement(element), }); this._loadURL(url, where, openParams, { diff --git a/browser/components/urlbar/UrlbarUtils.jsm b/browser/components/urlbar/UrlbarUtils.jsm index 2c7f77891253..1cd3198cf2b3 100644 --- a/browser/components/urlbar/UrlbarUtils.jsm +++ b/browser/components/urlbar/UrlbarUtils.jsm @@ -352,6 +352,11 @@ var UrlbarUtils = { ); return { url, postData }; } + 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 }; }, diff --git a/browser/components/urlbar/UrlbarView.jsm b/browser/components/urlbar/UrlbarView.jsm index 53097c44b50d..739211ea6f4f 100644 --- a/browser/components/urlbar/UrlbarView.jsm +++ b/browser/components/urlbar/UrlbarView.jsm @@ -235,6 +235,18 @@ class UrlbarView { return selectedRow.result; } + /** + * @returns {Element} + * The currently selected element. + */ + get selectedElement() { + if (!this.isOpen) { + return null; + } + + return this._selectedElement; + } + /** * @returns {number} * The number of visible results in the view. Note that this may be larger @@ -265,6 +277,27 @@ class UrlbarView { return sum; } + /** + * @param {Element} element + * An element in the view. + * @returns {UrlbarResult} + * The result attached to parameter `element`, if `element` is a row or a + * decendant of a row. + */ + getResultFromElement(element) { + if (!this.isOpen) { + return null; + } + + let row = this._getRowFromElement(element); + + if (!row) { + return null; + } + + return row.result; + } + /** * Moves the view selection forward or backward. * @@ -557,7 +590,6 @@ class UrlbarView { this._mainContainer.style.maxWidth = px(width); } - this.panel.removeAttribute("hidden"); this.input.inputField.setAttribute("aria-expanded", "true"); this.input.dropmarker.setAttribute("open", "true"); @@ -719,7 +751,7 @@ class UrlbarView { buttonSpacer.className = "urlbarView-tip-button-spacer"; content.appendChild(buttonSpacer); - let tipButton = this._createElement("button"); + let tipButton = this._createElement("span"); tipButton.className = "urlbarView-tip-button"; content.appendChild(tipButton); item._elements.set("tipButton", tipButton); @@ -1097,6 +1129,24 @@ class UrlbarView { return selected; } + /** + * @param {Element} element + * An element that is potentially a row or descendant of a row. + * @returns {Element} + * 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; + } + _setAccessibleFocus(item) { if (item) { this.input.inputField.setAttribute("aria-activedescendant", item.id); @@ -1259,12 +1309,7 @@ class UrlbarView { // Ignore right clicks. return; } - - let row = event.target; - while (!row.classList.contains("urlbarView-row")) { - row = row.parentNode; - } - this.input.pickResult(row.result, event); + this.input.pickElement(event.target, event); } _on_overflow(event) { diff --git a/browser/components/urlbar/tests/UrlbarTestUtils.jsm b/browser/components/urlbar/tests/UrlbarTestUtils.jsm index 497b719476a2..bc949c385f1a 100644 --- a/browser/components/urlbar/tests/UrlbarTestUtils.jsm +++ b/browser/components/urlbar/tests/UrlbarTestUtils.jsm @@ -173,7 +173,7 @@ var UrlbarTestUtils = { * @returns {HtmlElement|XulElement} The selected element. */ getSelectedElement(win) { - return win.gURLBar.view._selectedElement || null; + return win.gURLBar.view.selectedElement || null; }, /** diff --git a/browser/components/urlbar/tests/browser/browser.ini b/browser/components/urlbar/tests/browser/browser.ini index b2170ab44129..0149cea3d417 100644 --- a/browser/components/urlbar/tests/browser/browser.ini +++ b/browser/components/urlbar/tests/browser/browser.ini @@ -116,7 +116,7 @@ support-files = support-files = moz.png [browser_textruns.js] -[browser_tip_keyboard_selection.js] +[browser_tip_selection.js] [browser_urlbar_blanking.js] support-files = file_blank_but_not_blank.html diff --git a/browser/components/urlbar/tests/browser/browser_tip_keyboard_selection.js b/browser/components/urlbar/tests/browser/browser_tip_selection.js similarity index 76% rename from browser/components/urlbar/tests/browser/browser_tip_keyboard_selection.js rename to browser/components/urlbar/tests/browser/browser_tip_selection.js index ec8c453cc186..022e9bd314fa 100644 --- a/browser/components/urlbar/tests/browser/browser_tip_keyboard_selection.js +++ b/browser/components/urlbar/tests/browser/browser_tip_selection.js @@ -4,6 +4,8 @@ "use strict"; const MEGABAR_PREF = "browser.urlbar.megabar"; +const HELP_URL = "about:mozilla"; +const TIP_URL = "about:about"; // Tests keyboard selection within UrlbarUtils.RESULT_TYPE.TIP results. @@ -52,8 +54,8 @@ add_task(async function tipIsSecondResult() { text: "This is a test intervention.", buttonText: "Done", data: "test", - helpUrl: - "https://support.mozilla.org/en-US/kb/delete-browsing-search-download-history-firefox", + helpUrl: HELP_URL, + buttonUrl: TIP_URL, } ), ]; @@ -224,3 +226,56 @@ add_task(async function tipIsOnlyResult() { gURLBar.view.close(); UrlbarProvidersManager.unregisterProvider(provider); }); + +add_task(async function mouseSelection() { + window.windowUtils.disableNonTestMouseEvents(true); + registerCleanupFunction(() => { + window.windowUtils.disableNonTestMouseEvents(false); + }); + + let matches = [ + new UrlbarResult( + UrlbarUtils.RESULT_TYPE.TIP, + UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, + { + icon: "", + text: "This is a test intervention.", + buttonText: "Done", + data: "test", + helpUrl: HELP_URL, + buttonUrl: TIP_URL, + } + ), + ]; + + let provider = new TipTestProvider(matches); + UrlbarProvidersManager.registerProvider(provider); + + await UrlbarTestUtils.promiseAutocompleteResultPopup({ + value: "test", + window, + waitForFocus: SimpleTest.waitForFocus, + }); + let element = document.querySelector(".urlbarView-row .urlbarView-tip-help"); + let loadPromise = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); + EventUtils.synthesizeMouseAtCenter(element, {}, element.ownerGlobal); + await loadPromise; + Assert.equal( + gURLBar.value, + HELP_URL, + "Should have navigated to the tip's help page." + ); + + await UrlbarTestUtils.promiseAutocompleteResultPopup({ + value: "test", + window, + waitForFocus: SimpleTest.waitForFocus, + }); + element = document.querySelector(".urlbarView-row .urlbarView-tip-button"); + loadPromise = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); + EventUtils.synthesizeMouseAtCenter(element, {}, element.ownerGlobal); + await loadPromise; + Assert.equal(gURLBar.value, TIP_URL, "Should have navigated to the tip URL."); + + UrlbarProvidersManager.unregisterProvider(provider); +}); diff --git a/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry.js b/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry.js index 7c9312f2c61b..f9cbfe6fc0de 100644 --- a/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry.js +++ b/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry.js @@ -3,6 +3,27 @@ "use strict"; +XPCOMUtils.defineLazyModuleGetters(this, { + UrlbarProvider: "resource:///modules/UrlbarUtils.jsm", + UrlbarProvidersManager: "resource:///modules/UrlbarProvidersManager.jsm", + UrlbarUtils: "resource:///modules/UrlbarUtils.jsm", +}); + +function copyToClipboard(str) { + return new Promise((resolve, reject) => { + waitForClipboard( + str, + () => { + Cc["@mozilla.org/widget/clipboardhelper;1"] + .getService(Ci.nsIClipboardHelper) + .copyString(str); + }, + resolve, + reject + ); + }); +} + // Each test is a function that executes an urlbar action and returns the // expected event object, or null if no event is expected. const tests = [ @@ -145,6 +166,55 @@ const tests = [ }; }, + async function() { + let tipProvider = registerTipProvider(); + info("Selecting a tip's main button, enter."); + gURLBar.search("x"); + await promiseSearchComplete(); + EventUtils.synthesizeKey("KEY_ArrowDown"); + EventUtils.synthesizeKey("KEY_ArrowDown"); + EventUtils.synthesizeKey("VK_RETURN"); + unregisterTipProvider(tipProvider); + return { + category: "urlbar", + method: "engagement", + object: "enter", + value: "typed", + extra: { + elapsed: val => parseInt(val) > 0, + numChars: "1", + selIndex: "1", + selType: "tip", + }, + }; + }, + + async function() { + let tipProvider = registerTipProvider(); + info("Selecting a tip's help button, enter."); + let promise = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); + gURLBar.search("x"); + await promiseSearchComplete(); + EventUtils.synthesizeKey("KEY_ArrowDown"); + EventUtils.synthesizeKey("KEY_ArrowDown"); + EventUtils.synthesizeKey("KEY_ArrowDown"); + EventUtils.synthesizeKey("VK_RETURN"); + await promise; + unregisterTipProvider(tipProvider); + return { + category: "urlbar", + method: "engagement", + object: "enter", + value: "typed", + extra: { + elapsed: val => parseInt(val) > 0, + numChars: "1", + selIndex: "1", + selType: "tiphelp", + }, + }; + }, + async function() { info("Type something and canonize"); gURLBar.select(); @@ -736,3 +806,70 @@ add_task(async function test() { TelemetryTestUtils.assertEvents(expectedEvents, { category: "urlbar" }); } }); + +function registerTipProvider() { + let provider = new TipTestProvider(tipMatches); + UrlbarProvidersManager.registerProvider(provider); + return provider; +} + +function unregisterTipProvider(provider) { + UrlbarProvidersManager.unregisterProvider(provider); +} + +/** + * A test tip provider. See browser_tip_selection.js. + */ +class TipTestProvider extends UrlbarProvider { + constructor(matches) { + super(); + this._matches = matches; + } + get name() { + return "TipTestProvider"; + } + get type() { + return UrlbarUtils.PROVIDER_TYPE.PROFILE; + } + isActive(context) { + return true; + } + isRestricting(context) { + return true; + } + async startQuery(context, addCallback) { + this._context = context; + for (const match of this._matches) { + addCallback(this, match); + } + } + cancelQuery(context) {} +} + +let tipMatches = [ + new UrlbarResult( + UrlbarUtils.RESULT_TYPE.URL, + UrlbarUtils.RESULT_SOURCE.HISTORY, + { url: "http://mozilla.org/a" } + ), + new UrlbarResult( + UrlbarUtils.RESULT_TYPE.TIP, + UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, + { + text: "This is a test intervention.", + buttonText: "Done", + data: "test", + helpUrl: "about:about", + } + ), + new UrlbarResult( + UrlbarUtils.RESULT_TYPE.URL, + UrlbarUtils.RESULT_SOURCE.HISTORY, + { url: "http://mozilla.org/b" } + ), + new UrlbarResult( + UrlbarUtils.RESULT_TYPE.URL, + UrlbarUtils.RESULT_SOURCE.HISTORY, + { url: "http://mozilla.org/c" } + ), +]; diff --git a/browser/themes/shared/urlbar-autocomplete.inc.css b/browser/themes/shared/urlbar-autocomplete.inc.css index 5a4d2f7063a0..a16cc6b134b8 100644 --- a/browser/themes/shared/urlbar-autocomplete.inc.css +++ b/browser/themes/shared/urlbar-autocomplete.inc.css @@ -262,14 +262,15 @@ /* The tip button is a Photon default button when unfocused and a primary button in all other states. */ .urlbarView-tip-button { - min-height: 32px; + min-height: 16px; padding: 8px; border: none; border-radius: 2px; font-size: 0.93em; color: inherit; background-color: var(--in-content-button-background); - min-width: 10em; + min-width: 8.75em; + text-align: center; flex-shrink: 0; } diff --git a/toolkit/components/telemetry/Events.yaml b/toolkit/components/telemetry/Events.yaml index a2b8a71d6aae..fe6804affb5c 100644 --- a/toolkit/components/telemetry/Events.yaml +++ b/toolkit/components/telemetry/Events.yaml @@ -390,7 +390,7 @@ urlbar: type of the selected result in the urlbar panel. One of: "autofill", "visit", "bookmark", "history", "keyword", "search", "searchsuggestion", "switchtab", "remotetab", "extension", "oneoff", - "keywordoffer", "canonized", "none" + "keywordoffer", "canonized", "tip", "tiphelp", "none" abandonment: objects: ["blur"] release_channel_collection: opt-out