Bug 1498023 - Search shortcuts should hide all one-off UI when typed r=mak,Mardak

* Disable or enable the one-offs per each new search based on whether the first char is "@". The patch does this in `onResultsAdded`, where other per-search initialization happens.
* Remove the `disableOneOffButtons` option from the urlbar `search` method. It's not necessary anymore now that one-offs are automatically hidden for the only caller that uses this option (new tab with the "@engine" tiles).
* Make the `oneOffSearchesEnabled` getter return the actual status of the one-off UI instead of relying on an `_oneOffSearchesEnabled` property.

Differential Revision: https://phabricator.services.mozilla.com/D9883

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Drew Willcoxon 2018-10-29 16:13:49 +00:00
Родитель 7fe3db298f
Коммит f261cff897
7 изменённых файлов: 85 добавлений и 117 удалений

Просмотреть файл

@ -23,7 +23,8 @@ if (AppConstants.DEBUG ||
stack: [
"_rebuild@chrome://browser/content/search/search-one-offs.js",
"set popup@chrome://browser/content/search/search-one-offs.js",
"set_oneOffSearchesEnabled@chrome://browser/content/urlbarBindings.xml",
"_syncOneOffSearchesEnabled@chrome://browser/content/urlbarBindings.xml",
"toggleOneOffSearches@chrome://browser/content/urlbarBindings.xml",
"_enableOrDisableOneOffSearches@chrome://browser/content/urlbarBindings.xml",
"@chrome://browser/content/urlbarBindings.xml",
"_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",

Просмотреть файл

@ -24,7 +24,8 @@ if (AppConstants.DEBUG ||
stack: [
"_rebuild@chrome://browser/content/search/search-one-offs.js",
"set popup@chrome://browser/content/search/search-one-offs.js",
"set_oneOffSearchesEnabled@chrome://browser/content/urlbarBindings.xml",
"_syncOneOffSearchesEnabled@chrome://browser/content/urlbarBindings.xml",
"toggleOneOffSearches@chrome://browser/content/urlbarBindings.xml",
"_enableOrDisableOneOffSearches@chrome://browser/content/urlbarBindings.xml",
"@chrome://browser/content/urlbarBindings.xml",
"_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",

Просмотреть файл

@ -34,6 +34,8 @@ add_task(async function init() {
// Keys up and down through the history panel, i.e., the panel that's shown when
// there's no text in the textbox.
add_task(async function history() {
gURLBar.popup.toggleOneOffSearches(true);
gURLBar.focus();
EventUtils.synthesizeKey("KEY_ArrowDown");
await promisePopupShown(gURLBar.popup);
@ -249,6 +251,32 @@ add_task(async function collapsedOneOffs() {
await hidePopup();
});
// The one-offs should be hidden when searching with an "@engine" search engine
// alias.
add_task(async function hiddenWhenUsingSearchAlias() {
let typedValue = "@example";
await promiseAutocompleteResultPopup(typedValue, window, true);
await waitForAutocompleteResultAt(0);
Assert.equal(gURLBar.popup.oneOffSearchesEnabled, false);
Assert.equal(
window.getComputedStyle(gURLBar.popup.oneOffSearchButtons).display,
"none"
);
await hidePopup();
typedValue = "not an engine alias";
await promiseAutocompleteResultPopup(typedValue, window, true);
await waitForAutocompleteResultAt(0);
Assert.equal(gURLBar.popup.oneOffSearchesEnabled, true);
Assert.equal(
window.getComputedStyle(gURLBar.popup.oneOffSearchButtons).display,
"-moz-box"
);
await hidePopup();
});
function assertState(result, oneOff, textValue = undefined) {
Assert.equal(gURLBar.popup.selectedIndex, result,
"Expected result should be selected");

Просмотреть файл

@ -28,7 +28,7 @@ add_task(async function init() {
});
// Calls search() with only a search-string argument.
// Calls search() with a normal, non-"@engine" search-string argument.
add_task(async function basic() {
let resetNotification = enableSearchSuggestionsNotification();
@ -47,85 +47,15 @@ add_task(async function basic() {
});
// Calls search() with the search-suggestions notification disabled.
add_task(async function disableSearchSuggestionsNotification() {
// Calls search() with an "@engine" search engine alias so that the one-off
// search buttons and search-suggestions notification are disabled.
add_task(async function searchEngineAlias() {
let resetNotification = enableSearchSuggestionsNotification();
gURLBar.search("disableSearchSuggestionsNotification", {
disableSearchSuggestionsNotification: true,
});
gURLBar.search("@example");
await promiseSearchComplete();
await waitForAutocompleteResultAt(0);
assertUrlbarValue("disableSearchSuggestionsNotification");
assertSearchSuggestionsNotificationVisible(false);
assertOneOffButtonsVisible(true);
EventUtils.synthesizeKey("KEY_Escape");
await promisePopupHidden(gURLBar.popup);
// Open the popup again (by doing another search) to make sure the
// notification is shown -- i.e., that we didn't accidentally break it if
// it should continue being shown.
gURLBar.search("disableSearchSuggestionsNotification again");
await promiseSearchComplete();
await waitForAutocompleteResultAt(0);
assertUrlbarValue("disableSearchSuggestionsNotification again");
assertSearchSuggestionsNotificationVisible(true);
assertOneOffButtonsVisible(true);
EventUtils.synthesizeKey("KEY_Escape");
await promisePopupHidden(gURLBar.popup);
resetNotification();
});
// Calls search() with the one-off search buttons disabled.
add_task(async function disableOneOffButtons() {
let resetNotification = enableSearchSuggestionsNotification();
gURLBar.search("disableOneOffButtons", {
disableOneOffButtons: true,
});
await promiseSearchComplete();
await waitForAutocompleteResultAt(0);
assertUrlbarValue("disableOneOffButtons");
assertSearchSuggestionsNotificationVisible(true);
assertOneOffButtonsVisible(false);
EventUtils.synthesizeKey("KEY_Escape");
await promisePopupHidden(gURLBar.popup);
// Open the popup again (by doing another search) to make sure the one-off
// buttons are shown -- i.e., that we didn't accidentally break them.
gURLBar.search("disableOneOffButtons again");
await promiseSearchComplete();
await waitForAutocompleteResultAt(0);
assertUrlbarValue("disableOneOffButtons again");
assertSearchSuggestionsNotificationVisible(true);
assertOneOffButtonsVisible(true);
EventUtils.synthesizeKey("KEY_Escape");
await promisePopupHidden(gURLBar.popup);
resetNotification();
});
// Calls search() with both the search-suggestions notification and the one-off
// search buttons disabled.
add_task(async function disableSearchSuggestionsNotificationAndOneOffButtons() {
let resetNotification = enableSearchSuggestionsNotification();
gURLBar.search("disableSearchSuggestionsNotificationAndOneOffButtons", {
disableSearchSuggestionsNotification: true,
disableOneOffButtons: true,
});
await promiseSearchComplete();
await waitForAutocompleteResultAt(0);
assertUrlbarValue("disableSearchSuggestionsNotificationAndOneOffButtons");
assertUrlbarValue("@example");
assertSearchSuggestionsNotificationVisible(false);
assertOneOffButtonsVisible(false);
@ -136,10 +66,10 @@ add_task(async function disableSearchSuggestionsNotificationAndOneOffButtons() {
// Open the popup again (by doing another search) to make sure the
// notification and one-off buttons are shown -- i.e., that we didn't
// accidentally break them.
gURLBar.search("disableSearchSuggestionsNotificationAndOneOffButtons again");
gURLBar.search("not an engine alias");
await promiseSearchComplete();
await waitForAutocompleteResultAt(0);
assertUrlbarValue("disableSearchSuggestionsNotificationAndOneOffButtons again");
assertUrlbarValue("not an engine alias");
assertSearchSuggestionsNotificationVisible(true);
assertOneOffButtonsVisible(true);

Просмотреть файл

@ -1242,8 +1242,10 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
<method name="_enableOrDisableOneOffSearches">
<body><![CDATA[
this.popup.oneOffSearchesEnabled =
this._prefs.getBoolPref("oneOffSearches");
this.popup.toggleOneOffSearches(
this._prefs.getBoolPref("oneOffSearches"),
"pref"
);
]]></body>
</method>
@ -1642,32 +1644,13 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
@param value
The input's value will be set to this value, and the search will
use it as its query.
@param options
An optional object with the following optional properties:
* disableOneOffButtons: Set to true to hide the one-off search
buttons.
* disableSearchSuggestionsNotification: Set to true to hide the
onboarding opt-out search suggestions notification.
-->
<method name="search">
<parameter name="value"/>
<parameter name="options"/>
<body><![CDATA[
options = options || {};
if (options.disableOneOffButtons) {
this.popup.addEventListener("popupshowing", () => {
if (this.popup.oneOffSearchesEnabled) {
this.popup.oneOffSearchesEnabled = false;
this.popup.addEventListener("popuphidden", () => {
this.popup.oneOffSearchesEnabled = true;
}, {once: true});
}
}, {once: true});
}
if (options.disableSearchSuggestionsNotification &&
this.whichSearchSuggestionsNotification != "none") {
// Hide the suggestions notification if the search uses an "@engine"
// search engine alias.
if (value.trim()[0] == "@") {
let which = this.whichSearchSuggestionsNotification;
this._whichSearchSuggestionsNotification = "none";
this.popup.addEventListener("popuphidden", () => {
@ -1919,8 +1902,6 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
"one-off-search-buttons");
</field>
<field name="_oneOffSearchesEnabled">false</field>
<field name="_overrideValue">null</field>
<property name="overrideValue"
onget="return this._overrideValue;"
@ -1938,13 +1919,31 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
]]></body>
</method>
<property name="oneOffSearchesEnabled">
<getter><![CDATA[
return this._oneOffSearchesEnabled;
]]></getter>
<setter><![CDATA[
this._oneOffSearchesEnabled = !!val;
if (val) {
<field name="_oneOffSearchesEnabledByReason">new Map()</field>
<method name="toggleOneOffSearches">
<parameter name="enable"/>
<parameter name="reason"/>
<body><![CDATA[
this._oneOffSearchesEnabledByReason.set(reason || "runtime", enable);
this._syncOneOffSearchesEnabled();
]]></body>
</method>
<method name="_syncOneOffSearchesEnabled">
<body><![CDATA[
// If the popup hasn't ever been opened yet, then don't actually do
// anything. (The popup will still be hidden in that case.) The
// input adds a popupshowing listener that will call this method back
// and lazily initialize the one-off buttons the first time the popup
// opens. There are performance tests that fail if we don't do this.
if (this.hidden) {
return;
}
let enable = Array.from(this._oneOffSearchesEnabledByReason.values())
.every(v => v);
if (enable) {
this.oneOffSearchButtons.telemetryOrigin = "urlbar";
this.oneOffSearchButtons.style.display = "-moz-box";
// Set .textbox first, since the popup setter will cause
@ -1957,8 +1956,13 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
this.oneOffSearchButtons.textbox = null;
this.oneOffSearchButtons.popup = null;
}
return this._oneOffSearchesEnabled;
]]></setter>
]]></body>
</method>
<property name="oneOffSearchesEnabled" readonly="true">
<getter><![CDATA[
return this.oneOffSearchButtons.style.display != "none";
]]></getter>
</property>
<!-- Override this so that navigating between items results in an item
@ -2461,6 +2465,10 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
// selected and is now being reused.
if (selectHeuristic || !this.input.gotResultForCurrentQuery) {
this.input.formatValue();
// Also, hide the one-off search buttons if the user is using, or
// starting to use, an "@engine" search engine alias.
this.toggleOneOffSearches(this.input.value.trim()[0] != "@");
}
// If this is the first time we get the result from the current

Просмотреть файл

@ -280,7 +280,7 @@ class PlacesFeed {
}
fillSearchTopSiteTerm({_target, data}) {
_target.browser.ownerGlobal.gURLBar.search(`${data.label} `, {disableOneOffButtons: true, disableSearchSuggestionsNotification: true});
_target.browser.ownerGlobal.gURLBar.search(`${data.label} `);
}
onAction(action) {

Просмотреть файл

@ -250,7 +250,7 @@ describe("PlacesFeed", () => {
feed.fillSearchTopSiteTerm(action);
assert.calledOnce(locationBar.search);
assert.calledWithExactly(locationBar.search, "@Foo ", {disableOneOffButtons: true, disableSearchSuggestionsNotification: true});
assert.calledWithExactly(locationBar.search, "@Foo ");
});
it("should call saveToPocket on SAVE_TO_POCKET", () => {
const action = {