Bug 1709405: Make add engines compact. r=harry,desktop-theme-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D116057
This commit is contained in:
Daisuke Akatsuka
2021-06-03 01:48:25 +00:00
parent 5cc58b4694
commit 764d8c46cc
9 changed files with 110 additions and 384 deletions

View File

@@ -46,19 +46,18 @@ async function test_opensearch(shouldWork) {
searchBarButton.click();
await promiseSearchPopupShown;
let oneOffsContainer = searchPopup.searchOneOffsContainer;
let engineListElement = oneOffsContainer.querySelector(".search-add-engines");
let engineElement = oneOffsContainer.querySelector(
".searchbar-engine-one-off-add-engine"
);
if (shouldWork) {
ok(
engineListElement.firstElementChild,
"There should be search engines available to add"
);
ok(engineElement, "There should be search engines available to add");
ok(
searchBar.getAttribute("addengines"),
"Search bar should have addengines attribute"
);
} else {
is(
engineListElement.firstElementChild,
engineElement,
null,
"There should be no search engines available to add"
);

View File

@@ -10,13 +10,13 @@ const { XPCOMUtils } = ChromeUtils.import(
"resource://gre/modules/XPCOMUtils.jsm"
);
XPCOMUtils.defineLazyModuleGetters(this, {
clearTimeout: "resource://gre/modules/Timer.jsm",
PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
SearchUIUtils: "resource:///modules/SearchUIUtils.jsm",
Services: "resource://gre/modules/Services.jsm",
setTimeout: "resource://gre/modules/Timer.jsm",
});
const EMPTY_ADD_ENGINES = [];
/**
* Defines the search one-off button elements. These are displayed at the bottom
* of the address bar and search bar. The address bar buttons are a subclass in
@@ -40,7 +40,6 @@ class SearchOneOffs {
<hbox class="search-one-offs-spacer"/>
<button class="searchbar-engine-one-off-item search-setting-button-compact" tabindex="-1" data-l10n-id="search-one-offs-change-settings-compact-button"/>
</box>
<vbox class="search-add-engines"/>
<menuseparator class="searchbar-separator"/>
<button class="search-setting-button" pack="start" data-l10n-id="search-one-offs-change-settings-button"/>
<box>
@@ -73,8 +72,6 @@ class SearchOneOffs {
this.header = this.querySelector(".search-panel-one-offs-header");
this.addEngines = this.querySelector(".search-add-engines");
this.settingsButton = this.querySelector(".search-setting-button");
this.settingsButtonCompact = this.querySelector(
@@ -99,27 +96,7 @@ class SearchOneOffs {
*/
this._rebuilding = false;
/**
* If a page offers more than this number of engines, the add-engines
* menu button is shown, instead of showing the engines directly in the
* popup.
*/
this._addEngineMenuThreshold = 5;
/**
* All this stuff is to make the add-engines menu button behave like an
* actual menu. The add-engines menu button is shown when there are
* many engines offered by the current site.
*/
this._addEngineMenuTimeoutMs = 200;
this._addEngineMenuTimeout = null;
this._addEngineMenuShouldBeOpen = false;
this.addEventListener("mousedown", this);
this.addEventListener("mousemove", this);
this.addEventListener("mouseout", this);
this.addEventListener("click", this);
this.addEventListener("command", this);
this.addEventListener("contextmenu", this);
@@ -413,6 +390,14 @@ class SearchOneOffs {
this.invalidateCache();
}
_getAddEngines() {
return this.window.gBrowser.selectedBrowser.engines || EMPTY_ADD_ENGINES;
}
get _maxInlineAddEngines() {
return 3;
}
/**
* Infallible, non-re-entrant version of `__rebuild()`.
*/
@@ -435,19 +420,13 @@ class SearchOneOffs {
* Builds all the UI.
*/
async __rebuild() {
// Handle opensearch items. This needs to be done before building the
// list of one off providers, as that code will return early if all the
// alternative engines are hidden.
// Skip this in compact mode, ie. for the urlbar.
if (!this.compact) {
this._rebuildAddEngineList();
}
// Return early if the list of engines has not changed.
if (!this.popup && this._engineInfo?.domWasUpdated) {
return;
}
const addEngines = this._getAddEngines();
// Return early if the engines and panel width have not changed.
if (this.popup && this._textbox) {
let textboxWidth = await this.window.promiseDocumentFlushed(() => {
@@ -455,11 +434,13 @@ class SearchOneOffs {
});
if (
this._engineInfo?.domWasUpdated &&
this._textboxWidth == textboxWidth
this._textboxWidth == textboxWidth &&
this._addEngines == addEngines
) {
return;
}
this._textboxWidth = textboxWidth;
this._addEngines = addEngines;
}
// Finally, build the list of one-off buttons.
@@ -498,6 +479,7 @@ class SearchOneOffs {
}
let engines = (await this.getEngineInfo()).engines;
if (this.popup) {
let buttonsWidth = this.popup.clientWidth;
@@ -529,7 +511,8 @@ class SearchOneOffs {
// If the <div> with the list of search engines doesn't have
// a fixed height, the panel will be sized incorrectly, causing the bottom
// of the suggestion <tree> to be hidden.
let rowCount = Math.ceil(engines.length / enginesPerRow);
let engineCount = engines.length + addEngines.length;
let rowCount = Math.ceil(engineCount / enginesPerRow);
let height = rowCount * this.buttonHeight;
this.buttons.style.setProperty("height", `${height}px`);
}
@@ -538,7 +521,7 @@ class SearchOneOffs {
this.settingsButton.id = origin + "-anon-search-settings";
this.settingsButtonCompact.id = origin + "-anon-search-settings-compact";
this._rebuildEngineList(engines);
this._rebuildEngineList(engines, addEngines);
this.dispatchEvent(new Event("rebuild"));
}
@@ -547,8 +530,10 @@ class SearchOneOffs {
*
* @param {array} engines
* The engines to add.
* @param {array} addEngines
* The engines that can be added.
*/
_rebuildEngineList(engines) {
_rebuildEngineList(engines, addEngines) {
for (let i = 0; i < engines.length; ++i) {
let engine = engines[i];
let button = this.document.createXULElement("button");
@@ -563,102 +548,29 @@ class SearchOneOffs {
this.setTooltipForEngineButton(button);
this.buttons.appendChild(button);
}
}
_rebuildAddEngineList() {
let list = this.addEngines;
while (list.firstChild) {
list.firstChild.remove();
}
// Add a button for each engine that the page in the selected browser
// offers, except when there are too many offered engines.
// The popup isn't designed to handle too many (by scrolling for
// example), so a page could break the popup by offering too many.
// Instead, add a single menu button with a submenu of all the engines.
if (!this.window.gBrowser.selectedBrowser.engines) {
return;
}
let engines = this.window.gBrowser.selectedBrowser.engines;
let tooManyEngines = engines.length > this._addEngineMenuThreshold;
if (tooManyEngines) {
// Make the top-level menu button.
let button = this.document.createXULElement("toolbarbutton");
button.classList.add("addengine-menu-button", "addengine-item");
button.setAttribute("badged", "true");
button.setAttribute("type", "menu");
button.setAttribute("wantdropmarker", "true");
button.setAttribute("data-l10n-id", "search-one-offs-add-engine-menu");
button.setAttribute("crop", "end");
button.setAttribute("pack", "start");
// Set the menu button's image to the image of the first engine. The
// offered engines may have differing images, so there's no perfect
// choice here.
let engine = engines[0];
for (
let i = 0, len = Math.min(addEngines.length, this._maxInlineAddEngines);
i < len;
i++
) {
const engine = addEngines[i];
const button = this.document.createXULElement("button");
button.id = this._buttonIDForEngine(engine);
button.classList.add("searchbar-engine-one-off-item");
button.classList.add("searchbar-engine-one-off-add-engine");
button.setAttribute("tabindex", "-1");
if (engine.icon) {
button.setAttribute("image", engine.icon);
}
list.appendChild(button);
// Now make the button's child menupopup.
list = this.document.createXULElement("menupopup");
button.appendChild(list);
list.setAttribute("class", "addengine-menu");
list.setAttribute("position", "topright topleft");
// Events from child menupopups bubble up to the autocomplete binding,
// which breaks it, so prevent these events from propagating.
let suppressEventTypes = [
"popupshowing",
"popuphiding",
"popupshown",
"popuphidden",
];
for (let type of suppressEventTypes) {
list.addEventListener(type, event => {
event.stopPropagation();
});
}
}
// Finally, add the engines to the list. If there aren't too many
// engines, the list is the search-add-engines vbox. Otherwise it's the
// menupopup created earlier. In the latter case, create menuitem
// elements instead of buttons, because buttons don't get keyboard
// handling for free inside menupopups.
let eltType = tooManyEngines ? "menuitem" : "toolbarbutton";
for (let engine of engines) {
let button = this.document.createXULElement(eltType);
button.classList.add("addengine-item");
if (!tooManyEngines) {
button.setAttribute("badged", "true");
}
button.id =
this.telemetryOrigin +
"-add-engine-" +
this._fixUpEngineNameForID(engine.title);
button.setAttribute("data-l10n-id", "search-one-offs-add-engine");
button.setAttribute(
"data-l10n-args",
JSON.stringify({ engineName: engine.title })
);
button.setAttribute("crop", "end");
button.setAttribute("tooltiptext", engine.title + "\n" + engine.uri);
button.setAttribute("uri", engine.uri);
button.setAttribute("engine-name", engine.title);
if (engine.icon) {
button.setAttribute("image", engine.icon);
}
if (tooManyEngines) {
button.classList.add("menuitem-iconic");
} else {
button.setAttribute("pack", "start");
}
list.appendChild(button);
button.setAttribute("uri", engine.uri);
this.buttons.appendChild(button);
}
}
@@ -666,7 +578,7 @@ class SearchOneOffs {
return (
this.telemetryOrigin +
"-engine-one-off-item-" +
this._fixUpEngineNameForID(engine.name)
this._fixUpEngineNameForID(engine.name || engine.title)
);
}
@@ -680,23 +592,11 @@ class SearchOneOffs {
}
getSelectableButtons(aIncludeNonEngineButtons) {
let buttons = [];
for (
let oneOff = this.buttons.firstElementChild;
oneOff;
oneOff = oneOff.nextElementSibling
) {
buttons.push(oneOff);
}
const buttons = [
...this.buttons.querySelectorAll(".searchbar-engine-one-off-item"),
];
if (aIncludeNonEngineButtons) {
for (
let addEngine = this.addEngines.firstElementChild;
addEngine;
addEngine = addEngine.nextElementSibling
) {
buttons.push(addEngine);
}
buttons.push(
this.compact ? this.settingsButtonCompact : this.settingsButton
);
@@ -1056,17 +956,6 @@ class SearchOneOffs {
return false;
}
_resetAddEngineMenuTimeout() {
if (this._addEngineMenuTimeout) {
clearTimeout(this._addEngineMenuTimeout);
}
this._addEngineMenuTimeout = setTimeout(() => {
delete this._addEngineMenuTimeout;
let button = this.querySelector(".addengine-menu-button");
button.open = this._addEngineMenuShouldBeOpen;
}, this._addEngineMenuTimeoutMs);
}
// Methods for subclasses to override
/**
@@ -1137,44 +1026,12 @@ class SearchOneOffs {
// Event handlers below.
_on_mousedown(event) {
let target = event.originalTarget;
if (target.classList.contains("addengine-menu-button")) {
return;
}
// This is necessary to prevent the input from losing focus and closing the
// popup. Unfortunately it also has the side effect of preventing the
// buttons from receiving the `:active` pseudo-class.
event.preventDefault();
}
_on_mousemove(event) {
let target = event.originalTarget;
// Handle mouseover on the add-engine menu button and its popup items.
if (
(target.localName == "menuitem" &&
target.classList.contains("addengine-item")) ||
target.classList.contains("addengine-menu-button")
) {
this._addEngineMenuShouldBeOpen = true;
this._resetAddEngineMenuTimeout();
}
}
_on_mouseout(event) {
let target = event.originalTarget;
// Handle mouseout on the add-engine menu button and its popup items.
if (
(target.localName == "menuitem" &&
target.classList.contains("addengine-item")) ||
target.classList.contains("addengine-menu-button")
) {
this._addEngineMenuShouldBeOpen = false;
this._resetAddEngineMenuTimeout();
}
}
_on_click(event) {
if (event.button == 2) {
return; // ignore right clicks.
@@ -1205,10 +1062,7 @@ class SearchOneOffs {
return;
}
if (
target.classList.contains("addengine-item") ||
target.classList.contains("searchbar-engine-one-off-add-engine")
) {
if (target.classList.contains("searchbar-engine-one-off-add-engine")) {
// On success, hide the panel and tell event listeners to reshow it to
// show the new engine.
SearchUIUtils.addOpenSearchEngine(

View File

@@ -35,13 +35,16 @@ add_task(async function test_invalidEngine() {
);
await promise;
let addEngineList = searchPopup.querySelector(".search-add-engines");
let item = addEngineList.lastElementChild;
Assert.ok(
item.tooltipText.includes("engineInvalid"),
"Last item should be the invalid entry"
let addEngineList = searchPopup.querySelectorAll(
".searchbar-engine-one-off-add-engine"
);
let item = addEngineList[addEngineList.length - 1];
await TestUtils.waitForCondition(
() => item.tooltipText.includes("engineInvalid"),
"Wait until the tooltip will be correct"
);
Assert.ok(true, "Last item should be the invalid entry");
let promptPromise = PromptTestUtils.waitForPrompt(tab.linkedBrowser, {
modalType: Ci.nsIPromptService.MODAL_TYPE_CONTENT,

View File

@@ -1,7 +1,6 @@
// Tests that keyboard navigation in the search panel works as designed.
const searchPopup = document.getElementById("PopupSearchAutoComplete");
const oneOffsContainer = searchPopup.searchOneOffsContainer;
const kValues = ["foo1", "foo2", "foo3"];
const kUserValue = "foo";
@@ -9,7 +8,9 @@ const kUserValue = "foo";
function getOpenSearchItems() {
let os = [];
let addEngineList = oneOffsContainer.querySelector(".search-add-engines");
let addEngineList = searchPopup.searchOneOffsContainer.querySelector(
".search-add-engines"
);
for (
let item = addEngineList.firstElementChild;
item;
@@ -580,7 +581,9 @@ add_task(async function test_open_search() {
searchbar.focus();
await promise;
let engines = getOpenSearchItems();
let engines = searchPopup.querySelectorAll(
".searchbar-engine-one-off-add-engine"
);
is(engines.length, 3, "the opensearch.html page exposes 3 engines");
// Check that there's initially no selection.
@@ -604,16 +607,17 @@ add_task(async function test_open_search() {
"the engine #" + i + " should be selected"
);
ok(
selectedButton.classList.contains("addengine-item"),
"the button is themed as an engine item"
selectedButton.classList.contains("searchbar-engine-one-off-add-engine"),
"the button is themed as an add engine"
);
}
// Pressing up again should select the last one-off button.
EventUtils.synthesizeKey("KEY_ArrowUp");
const allOneOffs = getOneOffs();
is(
textbox.selectedButton,
getOneOffs().pop(),
allOneOffs[allOneOffs.length - engines.length - 1],
"the last one-off button should be selected"
);

View File

@@ -387,7 +387,9 @@ add_task(async function test_open_search() {
let engines;
await TestUtils.waitForCondition(() => {
engines = getOpenSearchItems();
engines = searchPopup.querySelectorAll(
".searchbar-engine-one-off-add-engine"
);
return engines.length == 3;
}, "Should expose three engines");
@@ -412,16 +414,17 @@ add_task(async function test_open_search() {
"the engine #" + i + " should be selected"
);
ok(
selectedButton.classList.contains("addengine-item"),
"the button is themed as an engine item"
selectedButton.classList.contains("searchbar-engine-one-off-add-engine"),
"the button is themed as an add engine"
);
}
// Pressing up again should select the last one-off button.
EventUtils.synthesizeKey("KEY_ArrowUp");
const allOneOffs = getOneOffs();
is(
textbox.selectedButton,
getOneOffs().pop(),
allOneOffs[allOneOffs.length - engines.length - 1],
"the last one-off button should be selected"
);

View File

@@ -1,8 +1,7 @@
"use strict";
// This test makes sure that when a page offers many search engines, the search
// popup shows a submenu that lists them instead of showing them in the popup
// itself.
// This test makes sure that when a page offers many search engines,
// a limited number of add-engine items will be shown in the searchbar.
const searchPopup = document.getElementById("PopupSearchAutoComplete");
@@ -34,77 +33,35 @@ add_task(async function test() {
EventUtils.synthesizeKey("KEY_ArrowDown");
await promise;
// Make sure it has only one add-engine menu button item.
let items = getOpenSearchItems();
Assert.equal(items.length, 1, "A single button");
let menuButton = items[0];
Assert.equal(menuButton.type, "menu", "A menu button");
await document.l10n.translateElements([menuButton]);
Assert.equal(menuButton.label, "Add search engine");
const addEngineList = searchPopup.oneOffButtons._getAddEngines();
Assert.equal(
addEngineList.length,
6,
"Expected number of engines retrieved from web page"
);
// Mouse over the menu button to open it.
let buttonPopup = menuButton.menupopup;
promise = promiseEvent(buttonPopup, "popupshown");
EventUtils.synthesizeMouse(menuButton, 5, 5, { type: "mousemove" });
await promise;
const displayedAddEngineList = searchPopup.oneOffButtons.buttons.querySelectorAll(
".searchbar-engine-one-off-add-engine"
);
Assert.equal(
displayedAddEngineList.length,
searchPopup.oneOffButtons._maxInlineAddEngines,
"Expected number of engines displayed on popup"
);
Assert.ok(menuButton.open, "Submenu should be open");
// Check the engines inside the submenu.
Assert.equal(buttonPopup.children.length, 6, "Expected number of engines");
for (let i = 0; i < buttonPopup.children.length; i++) {
let item = buttonPopup.children[i];
for (let i = 0; i < displayedAddEngineList.length; i++) {
const engine = addEngineList[i];
const item = displayedAddEngineList[i];
Assert.equal(
item.getAttribute("engine-name"),
"engine" + (i + 1),
"Expected engine title"
engine.title,
"Expected engine is displaying"
);
}
// Mouse out of the menu button to close it.
promise = promiseEvent(buttonPopup, "popuphidden");
EventUtils.synthesizeMouse(searchbar, 5, 5, { type: "mousemove" });
promise = promiseEvent(searchPopup, "popuphidden");
EventUtils.synthesizeKey("KEY_Escape", {}, searchPopup.ownerGlobal);
await promise;
Assert.ok(!menuButton.open, "Submenu should be closed");
// Key up until the menu button is selected.
for (
let button = null;
button != menuButton;
button = searchbar.textbox.popup.oneOffButtons.selectedButton
) {
EventUtils.synthesizeKey("KEY_ArrowUp");
}
// Press the Right arrow key. The submenu should open.
promise = promiseEvent(buttonPopup, "popupshown");
EventUtils.synthesizeKey("KEY_ArrowRight");
await promise;
Assert.ok(menuButton.open, "Submenu should be open");
// Press the Esc key. The submenu should close.
promise = promiseEvent(buttonPopup, "popuphidden");
EventUtils.synthesizeKey("KEY_Escape");
await promise;
Assert.ok(!menuButton.open, "Submenu should be closed");
gBrowser.removeCurrentTab();
});
function getOpenSearchItems() {
let os = [];
let addEngineList = searchPopup.oneOffButtons.addEngines;
for (
let item = addEngineList.firstElementChild;
item;
item = item.nextElementSibling
) {
os.push(item);
}
return os;
}

View File

@@ -3543,7 +3543,7 @@ class AddSearchEngineHelper {
* them in a submenu.
*/
get maxInlineEngines() {
return 3;
return this.shortcutButtons._maxInlineAddEngines;
}
/**
@@ -3556,21 +3556,7 @@ class AddSearchEngineHelper {
let engines = browser.engines?.slice() || [];
if (!this._sameEngines(this.engines, engines)) {
this.engines = engines;
// Update shortcut buttons. We only add `maxInlineEngines` engines, to
// cover the most common cases, others can be added from the context menu.
this.shortcutButtons.updateWebEngines(
engines.slice(0, this.maxInlineEngines).map(e => ({
name: e.title,
uri: e.uri,
get icon() {
// The icon is actually the browser favicon, that may not be in
// place yet, it will be fetched from the browser when the ui
// element is built.
return e.icon;
},
}))
);
this.shortcutButtons.updateWebEngines(engines);
}
}

View File

@@ -329,35 +329,25 @@ class UrlbarSearchOneOffs extends SearchOneOffs {
}
}
/**
* Overrides _getAddEngines to return engines that can be added.
*
* @returns {array} engines
*/
_getAddEngines() {
return this._webEngines;
}
/**
* Overrides _rebuildEngineList to add the local one-offs.
*
* @param {array} engines
* The search engines to add.
* @param {array} addEngines
* The engines that can be added.
*/
_rebuildEngineList(engines) {
super._rebuildEngineList(engines);
if (Services.prefs.getBoolPref("browser.proton.enabled", false)) {
for (let engine of this._webEngines) {
let button = this.document.createXULElement("button");
button.id = this._buttonIDForEngine(engine);
button.classList.add("searchbar-engine-one-off-item");
button.classList.add("searchbar-engine-one-off-add-engine");
button.setAttribute("tabindex", "-1");
if (engine.icon) {
button.setAttribute("image", engine.icon);
}
button.setAttribute("data-l10n-id", "search-one-offs-add-engine");
button.setAttribute(
"data-l10n-args",
JSON.stringify({ engineName: engine.name })
);
button.setAttribute("engine-name", engine.name);
button.setAttribute("uri", engine.uri);
this.buttons.appendChild(button);
}
}
_rebuildEngineList(engines, addEngines) {
super._rebuildEngineList(engines, addEngines);
for (let { source, pref, restrict } of UrlbarUtils.LOCAL_SEARCH_MODES) {
if (!UrlbarPrefs.get(pref)) {

View File

@@ -103,8 +103,7 @@
/* We don't handle `:active` because it doesn't work on the search or settings
buttons due to event.preventDefault() in SearchOneOffs._on_mousedown(). */
.searchbar-engine-one-off-item:not([selected]):hover,
.addengine-item:hover {
.searchbar-engine-one-off-item:not([selected]):hover {
background-color: var(--autocomplete-popup-hover-background);
color: inherit;
}
@@ -129,75 +128,6 @@
height: 16px;
}
.addengine-item {
appearance: none;
color: inherit;
height: 32px;
margin: var(--arrowpanel-menuitem-margin);
border-radius: var(--toolbarbutton-border-radius);
padding: 0 10px;
}
@media not (-moz-proton) {
.addengine-item {
border-radius: 0;
}
.addengine-item:first-of-type {
border-top: 1px solid var(--panel-separator-color);
}
} /*** END !proton ***/
.addengine-item[selected] {
background-color: var(--autocomplete-popup-highlight-background);
color: var(--autocomplete-popup-highlight-color);
}
.addengine-item[type=menu][selected] {
color: inherit;
background-color: var(--arrowpanel-dimmed-further);
}
.addengine-item > .toolbarbutton-badge-stack > .toolbarbutton-icon {
width: 16px;
height: 16px;
}
.addengine-item > .toolbarbutton-badge-stack > .toolbarbutton-badge {
display: -moz-box;
background: url(chrome://browser/skin/search-indicator-badge-add.svg) no-repeat center;
box-shadow: none;
/* "!important" is necessary to override the rule in toolbarbutton.css */
margin: -4px 0 0 !important;
margin-inline-end: -4px !important;
width: 11px;
height: 11px;
min-width: 11px;
min-height: 11px;
}
.addengine-item > .toolbarbutton-text {
text-align: start;
padding-inline-start: 10px;
}
.addengine-item:not([image]) {
list-style-image: url("chrome://browser/skin/search-engine-placeholder.png");
}
@media (min-resolution: 1.1dppx) {
.addengine-item:not([image]) {
list-style-image: url("chrome://browser/skin/search-engine-placeholder@2x.png");
}
}
.addengine-item[type=menu] > .toolbarbutton-menu-dropmarker {
display: -moz-box;
appearance: auto !important;
-moz-default-appearance: menuarrow !important;
list-style-image: none;
}
.search-panel-tree {
background: transparent;
color: inherit;