Bug 1384050 - Search suggestion contextual hint should only be shown when a user manually focuses the URL bar. r=paolo
MozReview-Commit-ID: IVbHYdtYJgx
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -2234,43 +2234,56 @@ function loadOneOrMoreURIs(aURIString, a
inBackground: false,
replace: true,
triggeringPrincipal: aTriggeringPrincipal,
});
} catch (e) {
}
}
-function focusAndSelectUrlBar() {
+/**
+ * Focuses the location bar input field and selects its contents.
+ *
+ * @param [optional] userInitiatedFocus
+ * Whether this focus is caused by an user interaction whose intention
+ * was to use the location bar. For example, using a shortcut to go to
+ * the location bar, or a contextual menu to search from it.
+ * The default is false and should be used in all those cases where the
+ * code focuses the location bar but that's not the primary user
+ * intention, like when opening a new tab.
+ */
+function focusAndSelectUrlBar(userInitiatedFocus = false) {
// In customize mode, the url bar is disabled. If a new tab is opened or the
// user switches to a different tab, this function gets called before we've
// finished leaving customize mode, and the url bar will still be disabled.
// We can't focus it when it's disabled, so we need to re-run ourselves when
// we've finished leaving customize mode.
if (CustomizationHandler.isExitingCustomizeMode) {
gNavToolbox.addEventListener("aftercustomization", function() {
- focusAndSelectUrlBar();
+ focusAndSelectUrlBar(userInitiatedFocus);
}, {once: true});
return true;
}
if (gURLBar) {
if (window.fullScreen)
FullScreen.showNavToolbox();
+ gURLBar.userInitiatedFocus = userInitiatedFocus;
gURLBar.select();
+ gURLBar.userInitiatedFocus = false;
if (document.activeElement == gURLBar.inputField)
return true;
}
return false;
}
function openLocation() {
- if (focusAndSelectUrlBar())
+ if (focusAndSelectUrlBar(true))
return;
if (window.location.href != getBrowserURL()) {
var win = getTopWin();
if (win) {
// If there's an open browser window, it should handle this command
win.focus()
win.openLocation();
@@ -3810,17 +3823,17 @@ const BrowserSearch = {
"chrome,all,dialog=no", "about:blank");
Services.obs.addObserver(observer, "browser-delayed-startup-finished");
}
return;
}
let focusUrlBarIfSearchFieldIsNotActive = function(aSearchBar) {
if (!aSearchBar || document.activeElement != aSearchBar.textbox.inputField) {
- focusAndSelectUrlBar();
+ focusAndSelectUrlBar(true);
}
};
let searchBar = this.searchBar;
let placement = CustomizableUI.getPlacementOfWidget("search-container");
let focusSearchBar = () => {
searchBar = this.searchBar;
searchBar.select();
--- a/browser/base/content/test/urlbar/browser_urlbarSearchSuggestions_opt-out.js
+++ b/browser/base/content/test/urlbar/browser_urlbarSearchSuggestions_opt-out.js
@@ -1,8 +1,9 @@
+/* eslint-disable mozilla/no-arbitrary-setTimeout */
// The order of the tests here matters!
const SUGGEST_ALL_PREF = "browser.search.suggest.enabled";
const SUGGEST_URLBAR_PREF = "browser.urlbar.suggest.searches";
const CHOICE_PREF = "browser.urlbar.userMadeSearchSuggestionsChoice";
const TIMES_PREF = "browser.urlbar.timesBeforeHidingSuggestionsHint";
const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
const ONEOFF_PREF = "browser.urlbar.oneOffSearches";
@@ -33,17 +34,17 @@ add_task(async function prepare() {
});
add_task(async function focus() {
// Focusing the urlbar should open the popup in order to show the
// notification.
setupVisibleHint();
gURLBar.blur();
let popupPromise = promisePopupShown(gURLBar.popup);
- gURLBar.focus();
+ focusAndSelectUrlBar(true);
await popupPromise;
Assert.ok(gURLBar.popup.popupOpen, "popup should be open");
assertVisible(true);
assertFooterVisible(false);
Assert.equal(gURLBar.popup._matchCount, 0, "popup should have no results");
// Start searching.
EventUtils.synthesizeKey("r", {});
@@ -59,31 +60,48 @@ add_task(async function focus() {
let prefsPromise = BrowserTestUtils.waitForLocationChange(gBrowser, "about:preferences#search");
changeOptionsLink.click();
await prefsPromise;
Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
// The preferences page does fancy stuff with focus, ensure to unload it.
await BrowserTestUtils.loadURI(gBrowser.selectedBrowser, "about:blank");
});
-add_task(async function new_tab() {
- // Opening a new tab when the urlbar is unfocused, should focusing it and thus
- // open the popup in order to show the notification.
+add_task(async function click_on_focused() {
+ // Even if the location bar is already focused, we should still show the popup
+ // and the notification on click.
setupVisibleHint();
gURLBar.blur();
+ // Won't show the hint since it's not user initiated.
+ gURLBar.focus();
+ await new Promise(resolve => setTimeout(resolve, 500));
+ Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
+
let popupPromise = promisePopupShown(gURLBar.popup);
- // openNewForegroundTab doesn't focus the urlbar.
- await BrowserTestUtils.synthesizeKey("t", { accelKey: true }, gBrowser.selectedBrowser);
+ EventUtils.synthesizeMouseAtCenter(gURLBar.inputField, { button: 0, type: "mousedown" });
await popupPromise;
+
Assert.ok(gURLBar.popup.popupOpen, "popup should be open");
assertVisible(true);
assertFooterVisible(false);
Assert.equal(gURLBar.popup._matchCount, 0, "popup should have no results");
+ gURLBar.blur();
+ Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
+});
+
+add_task(async function new_tab() {
+ // Opening a new tab when the urlbar is unfocused, should focus it but not
+ // open the popup.
+ setupVisibleHint();
+ gURLBar.blur();
+ // openNewForegroundTab doesn't focus the urlbar.
+ await BrowserTestUtils.synthesizeKey("t", { accelKey: true }, gBrowser.selectedBrowser);
+ await new Promise(resolve => setTimeout(resolve, 500));
+ Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
await BrowserTestUtils.removeTab(gBrowser.selectedTab);
- Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
});
add_task(async function privateWindow() {
// Since suggestions are disabled in private windows, the notification should
// not appear even when suggestions are otherwise enabled.
setupVisibleHint();
let win = await BrowserTestUtils.openNewBrowserWindow({ private: true });
await promiseAutocompleteResultPopup("foo", win);
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -183,16 +183,22 @@ file, You can obtain one at http://mozil
<!--
Since we never want scrollbars, we always use the maxResults value.
-->
<property name="maxRows"
onget="return this.popup.maxResults;"/>
<!--
+ Set by focusAndSelectUrlBar to indicate whether the next focus event was
+ initiated by an explicit user action. See the "focus" handler below.
+ -->
+ <field name="userInitiatedFocus">false</field>
+
+ <!--
onBeforeValueGet is called by the base-binding's .value getter.
It can return an object with a "value" property, to override the
return value of the getter.
-->
<method name="onBeforeValueGet">
<body><![CDATA[
return { value: this._value };
]]></body>
@@ -1448,16 +1454,37 @@ file, You can obtain one at http://mozil
let previousDate = this._prefs.getIntPref("lastSuggestionsPromptDate");
if (!useDays || previousDate != date) {
this._prefs.setIntPref(prefName, remaining - 1);
}
this._prefs.setIntPref("lastSuggestionsPromptDate", date);
]]></body>
</method>
+ <method name="maybeShowSearchSuggestionsNotificationOnFocus">
+ <parameter name="mouseFocused"/>
+ <body><![CDATA[
+ let whichNotification = this.whichSearchSuggestionsNotification;
+ if (this._showSearchSuggestionNotificationOnMouseFocus &&
+ mouseFocused) {
+ // Force showing the opt-out notification.
+ this._whichSearchSuggestionsNotification = whichNotification = "opt-out";
+ }
+ if (whichNotification == "opt-out") {
+ try {
+ this.popup.openAutocompletePopup(this, this);
+ } finally {
+ if (mouseFocused) {
+ delete this._whichSearchSuggestionsNotification;
+ this._showSearchSuggestionNotificationOnMouseFocus = false;
+ }
+ }
+ }
+ ]]></body>
+ </method>
</implementation>
<handlers>
<handler event="keydown"><![CDATA[
if (this._noActionKeys.has(event.keyCode) &&
this.popup.selectedIndex >= 0 &&
!this._pressedNoActionKeys.has(event.keyCode)) {
if (this._pressedNoActionKeys.size == 0) {
@@ -1472,52 +1499,51 @@ file, You can obtain one at http://mozil
if (this._noActionKeys.has(event.keyCode) &&
this._pressedNoActionKeys.has(event.keyCode)) {
this._pressedNoActionKeys.delete(event.keyCode);
if (this._pressedNoActionKeys.size == 0)
this._clearNoActions();
}
]]></handler>
+ <handler event="mousedown"><![CDATA[
+ // Eventually show the opt-out notification even if the location bar is
+ // empty, focused, and the user clicks on it.
+ if (event.button == 0 && this.focused && this.textValue == "") {
+ this.maybeShowSearchSuggestionsNotificationOnFocus(true);
+ }
+ ]]></handler>
+
<handler event="focus"><![CDATA[
if (event.originalTarget == this.inputField) {
this._hideURLTooltip();
this.formatValue();
if (this.getAttribute("pageproxystate") != "valid") {
UpdatePopupNotificationsVisibility();
}
- // We show the opt-out notification on every kind of focus to the urlbar
- // included opening a new tab, but we want to enforce at least one
+ // We show the opt-out notification when the mouse/keyboard focus the
+ // urlbar, but in any case we want to enforce at least one
// notification when the user focuses it with the mouse.
let whichNotification = this.whichSearchSuggestionsNotification;
if (whichNotification == "opt-out" &&
this._showSearchSuggestionNotificationOnMouseFocus === undefined) {
this._showSearchSuggestionNotificationOnMouseFocus = true;
}
- // Check whether the focus change came from a user mouse action.
+ // Check whether the focus change came from a keyboard/mouse action.
let focusMethod = Services.focus.getLastFocusMethod(window);
- let mouseFocused = !!(focusMethod & Services.focus.FLAG_BYMOUSE);
- if (this._showSearchSuggestionNotificationOnMouseFocus &&
- mouseFocused) {
- // Force showing the opt-out notification.
- this._whichSearchSuggestionsNotification = whichNotification = "opt-out";
+ // If it's a focus started by code and the primary user intention was
+ // not to go to the location bar, don't show a notification.
+ if (!focusMethod && !this.userInitiatedFocus) {
+ return;
}
- if (whichNotification == "opt-out") {
- try {
- this.popup.openAutocompletePopup(this, this);
- } finally {
- if (mouseFocused) {
- delete this._whichSearchSuggestionsNotification;
- this._showSearchSuggestionNotificationOnMouseFocus = false;
- }
- }
- }
+ let mouseFocused = !!(focusMethod & Services.focus.FLAG_BYMOUSE);
+ this.maybeShowSearchSuggestionsNotificationOnFocus(mouseFocused);
}
]]></handler>
<handler event="blur"><![CDATA[
if (event.originalTarget == this.inputField) {
this._clearNoActions();
this.formatValue();
if (this.getAttribute("pageproxystate") != "valid") {