diff --git a/browser/components/urlbar/UrlbarView.jsm b/browser/components/urlbar/UrlbarView.jsm index 5052f474fb83..a1d2e17d474a 100644 --- a/browser/components/urlbar/UrlbarView.jsm +++ b/browser/components/urlbar/UrlbarView.jsm @@ -20,13 +20,6 @@ XPCOMUtils.defineLazyModuleGetters(this, { // by setting UrlbarView.removeStaleRowsTimeout. const DEFAULT_REMOVE_STALE_ROWS_TIMEOUT = 400; -// The classNames of view elements that can be selected. -const SELECTABLE_ELEMENTS = [ - "urlbarView-row", - "urlbarView-tip-button", - "urlbarView-tip-help", -]; - const getBoundsWithoutFlushing = element => element.ownerGlobal.windowUtils.getBoundsWithoutFlushing(element); @@ -290,11 +283,13 @@ class UrlbarView { } /** + * Returns the result of the row containing the given element, or the result + * of the element if it itself is a row. + * * @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. + * The result of the element's row. */ getResultFromElement(element) { if (!this.isOpen) { @@ -310,6 +305,31 @@ class UrlbarView { return row.result; } + /** + * Returns the element closest to the given element that can be + * selected/picked. If the element itself can be selected, it's returned. If + * there is no such element, null is returned. + * + * @param {Element} element + * An element in the view. + * @returns {Element} + * The closest element that can be picked including the element itself, or + * null if there is no such element. + */ + getClosestSelectableElement(element) { + let result = this.getResultFromElement(element); + if (result && result.type == UrlbarUtils.RESULT_TYPE.TIP) { + if ( + element.classList.contains("urlbarView-tip-button") || + element.classList.contains("urlbarView-tip-help") + ) { + return element; + } + return null; + } + return element.closest(".urlbarView-row"); + } + /** * Moves the view selection forward or backward. * @@ -1436,11 +1456,12 @@ class UrlbarView { // Ignore right clicks. return; } - let target = event.target; - while (!SELECTABLE_ELEMENTS.includes(target.className)) { - target = target.parentNode; + let element = this.getClosestSelectableElement(event.target); + if (!element) { + // Ignore clicks on elements that can't be selected/picked. + return; } - this._selectElement(target, { updateInput: false }); + this._selectElement(element, { updateInput: false }); this.controller.speculativeConnect( this.selectedResult, this._queryContext, @@ -1453,7 +1474,12 @@ class UrlbarView { // Ignore right clicks. return; } - this.input.pickElement(event.target, event); + let element = this.getClosestSelectableElement(event.target); + if (!element) { + // Ignore clicks on elements that can't be selected/picked. + return; + } + this.input.pickElement(element, event); } _on_overflow(event) { diff --git a/browser/components/urlbar/tests/browser/browser_tip_selection.js b/browser/components/urlbar/tests/browser/browser_tip_selection.js index ae43958d61d5..454c107dbb11 100644 --- a/browser/components/urlbar/tests/browser/browser_tip_selection.js +++ b/browser/components/urlbar/tests/browser/browser_tip_selection.js @@ -1,14 +1,14 @@ /* Any copyright is dedicated to the Public Domain. * http://creativecommons.org/publicdomain/zero/1.0/ */ +// Tests keyboard selection within and clicks on UrlbarUtils.RESULT_TYPE.TIP +// results. + "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. - /** * A test provider. */ @@ -67,7 +67,7 @@ add_task(async function tipIsSecondResult() { await UrlbarTestUtils.promiseAutocompleteResultPopup({ value: "test", window, - waitForFocus: SimpleTest.waitForFocus, + waitForFocus, }); Assert.equal( @@ -120,14 +120,22 @@ add_task(async function tipIsSecondResult() { "The third element should be selected." ); - EventUtils.synthesizeKey("KEY_ArrowDown"); - await BrowserTestUtils.waitForCondition(() => { - info("Waiting for one-off to become selected."); - let oneOff = document.querySelector( - ".urlbarView .search-panel-one-offs .searchbar-engine-one-off-item:first-child" + // If this test is running alone, the one-offs will rebuild themselves when + // the view is opened above, and they may not be visible yet. Wait for the + // first one to become visible before trying to select it. + await TestUtils.waitForCondition(() => { + return ( + gURLBar.view.oneOffSearchButtons.buttons.firstElementChild && + BrowserTestUtils.is_visible( + gURLBar.view.oneOffSearchButtons.buttons.firstElementChild + ) ); - return oneOff.hasAttribute("selected"); - }); + }, "Waiting for first one-off to become visible."); + + EventUtils.synthesizeKey("KEY_ArrowDown"); + await TestUtils.waitForCondition(() => { + return gURLBar.view.oneOffSearchButtons.selectedButton; + }, "Waiting for one-off to become selected."); Assert.equal( UrlbarTestUtils.getSelectedElementIndex(window), -1, @@ -168,7 +176,7 @@ add_task(async function tipIsOnlyResult() { await UrlbarTestUtils.promiseAutocompleteResultPopup({ value: "test", window, - waitForFocus: SimpleTest.waitForFocus, + waitForFocus, }); Assert.equal( @@ -253,7 +261,7 @@ add_task(async function tipHasNoHelpButton() { await UrlbarTestUtils.promiseAutocompleteResultPopup({ value: "test", window, - waitForFocus: SimpleTest.waitForFocus, + waitForFocus, }); Assert.equal( @@ -289,13 +297,9 @@ add_task(async function tipHasNoHelpButton() { ); EventUtils.synthesizeKey("KEY_ArrowDown"); - await BrowserTestUtils.waitForCondition(() => { - info("Waiting for one-off to become selected."); - let oneOff = document.querySelector( - ".urlbarView .search-panel-one-offs .searchbar-engine-one-off-item:first-child" - ); - return oneOff.hasAttribute("selected"); - }); + await TestUtils.waitForCondition(() => { + return gURLBar.view.oneOffSearchButtons.selectedButton; + }, "Waiting for one-off to become selected."); Assert.equal( UrlbarTestUtils.getSelectedElementIndex(window), -1, @@ -338,31 +342,79 @@ add_task(async function mouseSelection() { let provider = new TipTestProvider(matches); UrlbarProvidersManager.registerProvider(provider); + // Click the help button. await UrlbarTestUtils.promiseAutocompleteResultPopup({ value: "test", window, - waitForFocus: SimpleTest.waitForFocus, + waitForFocus, }); - let element = document.querySelector(".urlbarView-row .urlbarView-tip-help"); + let row = await UrlbarTestUtils.waitForAutocompleteResultAt(window, 0); + let helpButton = row._elements.get("helpButton"); let loadPromise = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - EventUtils.synthesizeMouseAtCenter(element, {}, element.ownerGlobal); - await loadPromise; + await Promise.all([ + loadPromise, + UrlbarTestUtils.promisePopupClose(window, () => { + EventUtils.synthesizeMouseAtCenter(helpButton, {}); + }), + ]); Assert.equal( gURLBar.value, HELP_URL, "Should have navigated to the tip's help page." ); + // Click the main button. await UrlbarTestUtils.promiseAutocompleteResultPopup({ value: "test", window, - waitForFocus: SimpleTest.waitForFocus, + waitForFocus, }); - element = document.querySelector(".urlbarView-row .urlbarView-tip-button"); + row = await UrlbarTestUtils.waitForAutocompleteResultAt(window, 0); + let mainButton = row._elements.get("tipButton"); loadPromise = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - EventUtils.synthesizeMouseAtCenter(element, {}, element.ownerGlobal); - await loadPromise; + await Promise.all([ + loadPromise, + UrlbarTestUtils.promisePopupClose(window, () => { + EventUtils.synthesizeMouseAtCenter(mainButton, {}); + }), + ]); Assert.equal(gURLBar.value, TIP_URL, "Should have navigated to the tip URL."); + // Click inside the tip but outside the buttons. Nothing should happen. Make + // the result the heuristic to check that the selection on the main button + // isn't lost. + matches[0].heuristic = true; + await UrlbarTestUtils.promiseAutocompleteResultPopup({ + value: "test", + window, + waitForFocus, + }); + row = await UrlbarTestUtils.waitForAutocompleteResultAt(window, 0); + Assert.equal( + UrlbarTestUtils.getSelectedElementIndex(window), + 0, + "The main button's index should be selected initially" + ); + Assert.equal( + UrlbarTestUtils.getSelectedElement(window), + row._elements.get("tipButton"), + "The main button element should be selected initially" + ); + EventUtils.synthesizeMouseAtCenter(row, {}); + // eslint-disable-next-line mozilla/no-arbitrary-setTimeout + await new Promise(r => setTimeout(r, 500)); + Assert.ok(gURLBar.view.isOpen, "The view should remain open"); + Assert.equal( + UrlbarTestUtils.getSelectedElementIndex(window), + 0, + "The main button's index should remain selected" + ); + Assert.equal( + UrlbarTestUtils.getSelectedElement(window), + row._elements.get("tipButton"), + "The main button element should remain selected" + ); + await UrlbarTestUtils.promisePopupClose(window); + UrlbarProvidersManager.unregisterProvider(provider); });