Bug 1713827 - Hand off to search mode if searching is disabled in the Urlbar. r=amy,Standard8,flod

This patch also addresses bug 1645293. It essentially reverts parts 1 and 3 of bug 1616700 for users with searching disabled. Since we had to introduce branching behaviour, there are some new constructs like a shouldHandOffToSearchMode multi-pref in UrlbarPrefs.

Differential Revision: https://phabricator.services.mozilla.com/D118606
This commit is contained in:
Harry Twyford
2021-07-22 15:46:30 +00:00
parent 84ad45de17
commit 58cf2e7f96
16 changed files with 326 additions and 51 deletions

View File

@@ -27,6 +27,11 @@ ChromeUtils.defineModuleGetter(
"PlacesUtils",
"resource://gre/modules/PlacesUtils.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"PrivateBrowsingUtils",
"resource://gre/modules/PrivateBrowsingUtils.jsm"
);
const LINK_BLOCKED_EVENT = "newtab-linkBlocked";
const PLACES_LINKS_CHANGED_DELAY_TIME = 1000; // time in ms to delay timer for places links changed events
@@ -385,14 +390,23 @@ class PlacesFeed {
});
}
_getDefaultSearchEngine(isPrivateWindow) {
return Services.search[
isPrivateWindow ? "defaultPrivateEngine" : "defaultEngine"
];
}
handoffSearchToAwesomebar({ _target, data, meta }) {
const searchEngine = this._getDefaultSearchEngine(
PrivateBrowsingUtils.isBrowserPrivate(_target.browser)
);
const urlBar = _target.browser.ownerGlobal.gURLBar;
let isFirstChange = true;
if (!data || !data.text) {
urlBar.setHiddenFocus();
} else {
urlBar.search(data.text);
urlBar.handoff(data.text, searchEngine);
isFirstChange = false;
}
@@ -403,7 +417,7 @@ class PlacesFeed {
if (isFirstChange) {
isFirstChange = false;
urlBar.removeHiddenFocus(true);
urlBar.search("");
urlBar.handoff("", searchEngine);
this.store.dispatch(
ac.OnlyToOneContent({ type: at.DISABLE_SEARCH }, meta.fromTarget)
);

View File

@@ -653,7 +653,7 @@ describe("PlacesFeed", () => {
beforeEach(() => {
fakeUrlBar = {
focus: sinon.spy(),
search: sinon.spy(),
handoff: sinon.spy(),
setHiddenFocus: sinon.spy(),
removeHiddenFocus: sinon.spy(),
addEventListener: (ev, cb) => {
@@ -670,12 +670,12 @@ describe("PlacesFeed", () => {
meta: { fromTarget: {} },
});
assert.calledOnce(fakeUrlBar.setHiddenFocus);
assert.notCalled(fakeUrlBar.search);
assert.notCalled(fakeUrlBar.handoff);
assert.notCalled(feed.store.dispatch);
// Now type a character.
listeners.keydown({ key: "f" });
assert.calledOnce(fakeUrlBar.search);
assert.calledOnce(fakeUrlBar.handoff);
assert.calledOnce(fakeUrlBar.removeHiddenFocus);
assert.calledOnce(feed.store.dispatch);
assert.calledWith(feed.store.dispatch, {
@@ -694,8 +694,12 @@ describe("PlacesFeed", () => {
data: { text: "foo" },
meta: { fromTarget: {} },
});
assert.calledOnce(fakeUrlBar.search);
assert.calledWith(fakeUrlBar.search, "foo");
assert.calledOnce(fakeUrlBar.handoff);
assert.calledWith(
fakeUrlBar.handoff,
"foo",
global.Services.search.defaultEngine
);
assert.notCalled(fakeUrlBar.focus);
assert.notCalled(fakeUrlBar.setHiddenFocus);
@@ -719,8 +723,12 @@ describe("PlacesFeed", () => {
data: { text: "foo" },
meta: { fromTarget: {} },
});
assert.calledOnce(fakeUrlBar.search);
assert.calledWith(fakeUrlBar.search, "foo");
assert.calledOnce(fakeUrlBar.handoff);
assert.calledWith(
fakeUrlBar.handoff,
"foo",
global.Services.search.defaultPrivateEngine
);
assert.notCalled(fakeUrlBar.focus);
assert.notCalled(fakeUrlBar.setHiddenFocus);
@@ -744,8 +752,12 @@ describe("PlacesFeed", () => {
data: { text: "foo" },
meta: { fromTarget: {} },
});
assert.calledOnce(fakeUrlBar.search);
assert.calledWithExactly(fakeUrlBar.search, "foo");
assert.calledOnce(fakeUrlBar.handoff);
assert.calledWithExactly(
fakeUrlBar.handoff,
"foo",
global.Services.search.defaultEngine
);
assert.notCalled(fakeUrlBar.focus);
// Now call ESC keydown.

View File

@@ -240,30 +240,38 @@ document.addEventListener("DOMContentLoaded", function() {
// Setup the search hand-off box.
let btn = document.getElementById("search-handoff-button");
RPMSendQuery("ShouldShowSearch", {}).then(engineName => {
let input = document.querySelector(".fake-textbox");
if (engineName) {
document.l10n.setAttributes(btn, "about-private-browsing-handoff", {
engine: engineName,
});
document.l10n.setAttributes(
input,
"about-private-browsing-handoff-text",
{
RPMSendQuery("ShouldShowSearch", {}).then(
([engineName, shouldHandOffToSearchMode]) => {
let input = document.querySelector(".fake-textbox");
if (shouldHandOffToSearchMode) {
document.l10n.setAttributes(btn, "about-private-browsing-search-btn");
document.l10n.setAttributes(
input,
"about-private-browsing-search-placeholder"
);
} else if (engineName) {
document.l10n.setAttributes(btn, "about-private-browsing-handoff", {
engine: engineName,
}
);
} else {
document.l10n.setAttributes(
btn,
"about-private-browsing-handoff-no-engine"
);
document.l10n.setAttributes(
input,
"about-private-browsing-handoff-text-no-engine"
);
});
document.l10n.setAttributes(
input,
"about-private-browsing-handoff-text",
{
engine: engineName,
}
);
} else {
document.l10n.setAttributes(
btn,
"about-private-browsing-handoff-no-engine"
);
document.l10n.setAttributes(
input,
"about-private-browsing-handoff-text-no-engine"
);
}
}
});
);
let editable = document.getElementById("fake-editable");
let DISABLE_SEARCH_TOPIC = "DisableSearch";

View File

@@ -37,7 +37,12 @@ let expectedIconURL;
add_task(async function setup() {
await SpecialPowers.pushPrefEnv({
set: [["browser.search.separatePrivateDefault", true]],
set: [
["browser.search.separatePrivateDefault", true],
// Enable suggestions in this test. Otherwise, the behaviour of the
// content search box changes.
["browser.search.suggest.enabled", true],
],
});
const originalPrivateDefault = await Services.search.getDefaultPrivate();
@@ -193,3 +198,61 @@ add_task(async function test_search_handoff_on_paste() {
await BrowserTestUtils.closeWindow(win);
});
/**
* Tests that handoff enters search mode when suggestions are disabled.
*/
add_task(async function test_search_handoff_search_mode() {
await SpecialPowers.pushPrefEnv({
set: [["browser.urlbar.suggest.searches", false]],
});
let { win, tab } = await openAboutPrivateBrowsing();
await SpecialPowers.spawn(tab, [], async function() {
let btn = content.document.getElementById("search-handoff-button");
btn.click();
ok(btn.classList.contains("focused"), "in-content search has focus styles");
});
ok(urlBarHasHiddenFocus(win), "Urlbar has hidden focus");
// Expect two searches, one to enter search mode and then another in search
// mode.
let searchPromise = UrlbarTestUtils.promiseSearchComplete(win);
await new Promise(r => EventUtils.synthesizeKey("f", {}, win, r));
await SpecialPowers.spawn(tab, [], async function() {
ok(
content.document
.getElementById("search-handoff-button")
.classList.contains("disabled"),
"in-content search is disabled"
);
});
await searchPromise;
ok(urlBarHasNormalFocus(win), "Urlbar has normal focus");
await UrlbarTestUtils.assertSearchMode(win, {
engineName: "DuckDuckGo",
source: UrlbarUtils.RESULT_SOURCE.SEARCH,
entry: "handoff",
});
is(win.gURLBar.value, "f", "url bar has search text");
// Close the popup.
await UrlbarTestUtils.exitSearchMode(win);
await UrlbarTestUtils.promisePopupClose(win);
// Hitting ESC should reshow the in-content search
await new Promise(r => EventUtils.synthesizeKey("KEY_Escape", {}, win, r));
await SpecialPowers.spawn(tab, [], async function() {
ok(
!content.document
.getElementById("search-handoff-button")
.classList.contains("disabled"),
"in-content search is not disabled"
);
});
await BrowserTestUtils.closeWindow(win);
await SpecialPowers.popPrefEnv();
});

View File

@@ -10,6 +10,7 @@ function ContentSearchHandoffUIController() {
window.addEventListener("ContentSearchService", this);
this._sendMsg("GetEngine");
this._sendMsg("GetHandoffSearchModePrefs");
}
ContentSearchHandoffUIController.prototype = {
@@ -41,6 +42,11 @@ ContentSearchHandoffUIController.prototype = {
}
},
_onMsgHandoffSearchModePrefs(pref) {
this._shouldHandOffToSearchMode = pref;
this._updatel10nIds();
},
_updateEngine(engine) {
this._defaultEngine = engine;
if (this._engineIcon) {
@@ -61,10 +67,30 @@ ContentSearchHandoffUIController.prototype = {
"--newtab-search-icon",
"url(" + this._engineIcon + ")"
);
this._updatel10nIds();
},
_updatel10nIds() {
let engine = this._defaultEngine;
let fakeButton = document.querySelector(".search-handoff-button");
let fakeInput = document.querySelector(".fake-textbox");
if (!engine.isAppProvided) {
if (!fakeButton || !fakeInput) {
return;
}
if (!engine || this._shouldHandOffToSearchMode) {
document.l10n.setAttributes(
fakeButton,
this._isPrivateWindow
? "about-private-browsing-search-btn"
: "newtab-search-box-input"
);
document.l10n.setAttributes(
fakeInput,
this._isPrivateWindow
? "about-private-browsing-search-placeholder"
: "newtab-search-box-text"
);
} else if (!engine.isAppProvided) {
document.l10n.setAttributes(
fakeButton,
this._isPrivateWindow

View File

@@ -22,6 +22,12 @@ add_task(async function setup() {
addedEngine = await Services.search.getEngineByName(TEST_ENGINE_NAME);
// Enable suggestions in this test. Otherwise, the string in the content
// search box changes.
await SpecialPowers.pushPrefEnv({
set: [["browser.urlbar.suggest.searches", true]],
});
registerCleanupFunction(async () => {
await Services.search.setDefault(defaultEngine);
});
@@ -96,25 +102,37 @@ async function runNewTabTest(isHandoff) {
await ensurePlaceholder(tab, "newtab-search-box-handoff-input-no-engine");
}
// Disable suggestions in the Urlbar. This should update the placeholder
// string since handoff will now enter search mode.
if (isHandoff) {
await SpecialPowers.pushPrefEnv({
set: [["browser.urlbar.suggest.searches", false]],
});
await ensurePlaceholder(tab, "newtab-search-box-input");
await SpecialPowers.popPrefEnv();
}
await Services.search.setDefault(defaultEngine);
BrowserTestUtils.removeTab(tab);
}
add_task(async function test_content_search_attributes() {
SpecialPowers.pushPrefEnv({
await SpecialPowers.pushPrefEnv({
set: [[HANDOFF_PREF, true]],
});
await runNewTabTest(true);
await SpecialPowers.popPrefEnv();
});
add_task(async function test_content_search_attributes_no_handoff() {
SpecialPowers.pushPrefEnv({
await SpecialPowers.pushPrefEnv({
set: [[HANDOFF_PREF, false]],
});
await runNewTabTest(false);
await SpecialPowers.popPrefEnv();
});
add_task(async function test_content_search_attributes_in_private_window() {
@@ -140,6 +158,12 @@ add_task(async function test_content_search_attributes_in_private_window() {
await ensureIcon(tab, "chrome://global/skin/icons/search-glass.svg");
await ensurePlaceholder(tab, "about-private-browsing-handoff-no-engine");
await SpecialPowers.pushPrefEnv({
set: [["browser.urlbar.suggest.searches", false]],
});
await ensurePlaceholder(tab, "about-private-browsing-search-btn");
await SpecialPowers.popPrefEnv();
await Services.search.setDefault(defaultEngine);
await BrowserTestUtils.closeWindow(win);

View File

@@ -660,6 +660,28 @@ class UrlbarInput {
}
}
/**
* Called by inputs that resemble search boxes, but actually hand input off
* to the Urlbar. We use these fake inputs on the new tab page and
* about:privatebrowsing.
*
* @param {string} searchString
* @param {nsISearchEngine} [searchEngine]
* Optional. If included and the right prefs are set, we will enter search
* mode when handing `searchString` from the fake input to the Urlbar.
*
*/
handoff(searchString, searchEngine) {
if (UrlbarPrefs.get("shouldHandOffToSearchMode") && searchEngine) {
this.search(searchString, {
searchEngine,
searchModeEntry: "handoff",
});
} else {
this.search(searchString);
}
}
/**
* Called when an element of the view is picked.
*

View File

@@ -402,6 +402,13 @@ class Preferences {
}
this._observerWeakRefs = [];
this.addObserver(this);
// These prefs control the value of the shouldHandOffToSearchMode pref. They
// are exposed as a class variable so UrlbarPrefs observers can watch for
// changes in these prefs.
this.shouldHandOffToSearchModePrefs = [
"keyword.enabled",
"suggest.searches",
];
NimbusFeatures.urlbar.onUpdate(() => this._onNimbusUpdate());
}
@@ -544,6 +551,10 @@ class Preferences {
if (pref.startsWith("suggest.")) {
this._map.delete("defaultBehavior");
}
if (this.shouldHandOffToSearchModePrefs.includes(pref)) {
this._map.delete("shouldHandOffToSearchMode");
}
}
/**
@@ -608,6 +619,10 @@ class Preferences {
return makeResultBuckets({
showSearchSuggestionsFirst: this.get("showSearchSuggestionsFirst"),
});
case "shouldHandOffToSearchMode":
return this.shouldHandOffToSearchModePrefs.some(
prefName => !this.get(prefName)
);
}
return this._readPref(pref);
}

View File

@@ -197,6 +197,7 @@ var UrlbarUtils = {
// telemetry documentation and Scalars.yaml.
SEARCH_MODE_ENTRY: new Set([
"bookmarkmenu",
"handoff",
"keywordoffer",
"oneoff",
"other",

View File

@@ -135,8 +135,9 @@ urlbar.searchmode.*
menu.
- ``handoff``
Used when the user uses the search box on the new tab page and is handed off
to the address bar. NOTE: This entry point was deprecated in Firefox 88.
Handoff no longer enters search mode.
to the address bar. NOTE: This entry point was disabled from Firefox 88 to
91. Starting with 91, it will appear but in low volume. Users must have
searching in the Urlbar disabled to enter search mode via handoff.
- ``keywordoffer``
Used when the user selects a keyword offer result.
- ``oneoff``

View File

@@ -38,8 +38,13 @@ class AwaitPromiseProvider extends UrlbarTestUtils.TestProvider {
}
add_task(async function setup() {
await SearchTestUtils.installSearchExtension();
let engine = Services.search.getEngineByName("Example");
let oldDefaultEngine = Services.search.defaultEngine;
Services.search.defaultEngine = engine;
registerCleanupFunction(function() {
SpecialPowers.clipboardCopyString("");
Services.search.defaultEngine = oldDefaultEngine;
});
});

View File

@@ -392,6 +392,44 @@ add_task(async function test_tabmenu() {
assertSearchModeScalars("tabmenu", "tabs");
});
// Enters search mode by performing a search handoff on about:privatebrowsing.
// Note that handoff-to-search-mode only occurs when suggestions are disabled
// in the Urlbar.
// NOTE: We don't test handoff on about:home. Running mochitests on about:home
// is quite difficult. This subtest verifies that `handoff` is a valid scalar
// suffix and that a call to UrlbarInput.handoff(value, searchEngine) records
// values in the urlbar.searchmode.handoff scalar. PlacesFeed.test.js verfies that
// about:home handoff makes that exact call.
add_task(async function test_handoff_pbm() {
await SpecialPowers.pushPrefEnv({
set: [["browser.urlbar.suggest.searches", false]],
});
let win = await BrowserTestUtils.openNewBrowserWindow({
private: true,
waitForTabURL: "about:privatebrowsing",
});
let tab = win.gBrowser.selectedBrowser;
await SpecialPowers.spawn(tab, [], async function() {
let btn = content.document.getElementById("search-handoff-button");
btn.click();
});
let searchPromise = UrlbarTestUtils.promiseSearchComplete(win);
await new Promise(r => EventUtils.synthesizeKey("f", {}, win, r));
await searchPromise;
await UrlbarTestUtils.assertSearchMode(win, {
engineName,
entry: "handoff",
});
assertSearchModeScalars("handoff", "other");
await UrlbarTestUtils.exitSearchMode(win);
await UrlbarTestUtils.promisePopupClose(win);
await BrowserTestUtils.closeWindow(win);
await SpecialPowers.popPrefEnv();
});
// Enters search mode by tapping a search shortcut on the Touch Bar.
add_task(async function test_touchbar() {
if (AppConstants.platform != "macosx") {