Bug 1482610 - Part 2 - Move the remaining attributes and remove the sidebar broadcasters. r=jaws,mixedpuppy

This also fixes unintended behavior for which clicking the selected item in the sidebar selector would hide the sidebar.

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

--HG--
extra : rebase_source : 891c99ab68f4689513801a1957a3d3846b7ffe58
This commit is contained in:
Paolo Amadini 2018-08-19 19:54:02 +01:00
Родитель 327194f39f
Коммит 72ec57e0b2
9 изменённых файлов: 110 добавлений и 121 удалений

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

@ -199,16 +199,19 @@
accesskey="&viewSidebarMenu.accesskey;">
<menupopup id="viewSidebarMenu">
<menuitem id="menu_bookmarksSidebar"
type="checkbox"
key="viewBookmarksSidebarKb"
observes="viewBookmarksSidebar"
oncommand="SidebarUI.toggle('viewBookmarksSidebar');"
label="&bookmarksButton.label;"/>
<menuitem id="menu_historySidebar"
type="checkbox"
key="key_gotoHistory"
observes="viewHistorySidebar"
oncommand="SidebarUI.toggle('viewHistorySidebar');"
label="&historyButton.label;"/>
<menuitem id="menu_tabsSidebar"
type="checkbox"
class="sync-ui-item"
observes="viewTabsSidebar"
oncommand="SidebarUI.toggle('viewTabsSidebar');"
label="&syncedTabs.sidebar.label;"/>
</menupopup>
</menu>

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

@ -122,14 +122,6 @@
#include ../../components/places/content/placesCommands.inc.xul
<broadcasterset id="mainBroadcasterSet">
<broadcaster id="viewBookmarksSidebar"
type="checkbox" group="sidebar"
oncommand="SidebarUI.toggle('viewBookmarksSidebar');"/>
<broadcaster id="viewHistorySidebar"
type="checkbox" group="sidebar"
oncommand="SidebarUI.toggle('viewHistorySidebar');"/>
<broadcaster id="isImage"/>
<broadcaster id="canViewSource"/>
<broadcaster id="isFrameImage"/>
@ -145,9 +137,6 @@
<broadcaster id="sync-unverified-state" hidden="true"/>
<broadcaster id="sync-syncnow-state" hidden="true"/>
<broadcaster id="sync-reauth-state" hidden="true"/>
<broadcaster id="viewTabsSidebar"
type="checkbox" group="sidebar"
oncommand="SidebarUI.toggle('viewTabsSidebar');"/>
<broadcaster id="workOfflineMenuitemState"/>
<broadcaster id="devtoolsMenuBroadcaster_RecordExecution"
label="&devtoolsRecordExecution.label;"
@ -286,11 +275,11 @@
#else
<key id="manBookmarkKb" key="&bookmarksGtkCmd.commandkey;" command="Browser:ShowAllBookmarks" modifiers="accel,shift"/>
#endif
<key id="viewBookmarksSidebarKb" key="&bookmarksCmd.commandkey;" command="viewBookmarksSidebar" modifiers="accel"/>
<key id="viewBookmarksSidebarKb" key="&bookmarksCmd.commandkey;" oncommand="SidebarUI.toggle('viewBookmarksSidebar');" modifiers="accel"/>
#ifdef XP_WIN
# Cmd+I is conventially mapped to Info on MacOS X, thus it should not be
# overridden for other purposes there.
<key id="viewBookmarksSidebarWinKb" key="&bookmarksWinCmd.commandkey;" command="viewBookmarksSidebar" modifiers="accel"/>
<key id="viewBookmarksSidebarWinKb" key="&bookmarksWinCmd.commandkey;" oncommand="SidebarUI.toggle('viewBookmarksSidebar');" modifiers="accel"/>
#endif
<key id="key_stop" keycode="VK_ESCAPE" command="Browser:Stop"/>
@ -306,7 +295,7 @@
#else
modifiers="accel"
#endif
command="viewHistorySidebar"/>
oncommand="SidebarUI.toggle('viewHistorySidebar');"/>
<key id="key_fullZoomReduce" key="&fullZoomReduceCmd.commandkey;" command="cmd_fullZoomReduce" modifiers="accel"/>
<key key="&fullZoomReduceCmd.commandkey2;" command="cmd_fullZoomReduce" modifiers="accel"/>

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

@ -15,16 +15,22 @@ var SidebarUI = {
title: document.getElementById("sidebar-switcher-bookmarks")
.getAttribute("label"),
url: "chrome://browser/content/places/bookmarksSidebar.xul",
menuId: "menu_bookmarksSidebar",
buttonId: "sidebar-switcher-bookmarks",
}],
["viewHistorySidebar", {
title: document.getElementById("sidebar-switcher-history")
.getAttribute("label"),
url: "chrome://browser/content/places/historySidebar.xul",
menuId: "menu_historySidebar",
buttonId: "sidebar-switcher-history",
}],
["viewTabsSidebar", {
title: document.getElementById("sidebar-switcher-tabs")
.getAttribute("label"),
url: "chrome://browser/content/syncedtabs/sidebar.xhtml",
menuId: "menu_tabsSidebar",
buttonId: "sidebar-switcher-tabs",
}],
]);
},
@ -235,7 +241,7 @@ var SidebarUI = {
// dynamically generated sidebars will fail this check, but we still
// consider it adopted.
if (!document.getElementById(commandID)) {
if (!this.sidebars.has(commandID)) {
return true;
}
@ -275,7 +281,7 @@ var SidebarUI = {
}
let commandID = this._box.getAttribute("sidebarcommand");
if (commandID && document.getElementById(commandID)) {
if (commandID && this.sidebars.has(commandID)) {
this.showInitially(commandID);
} else {
this._box.removeAttribute("checked");
@ -323,7 +329,7 @@ var SidebarUI = {
},
/**
* The ID of the current sidebar (ie, the ID of the broadcaster being used).
* The ID of the current sidebar.
*/
get currentID() {
return this.isOpen ? this._box.getAttribute("sidebarcommand") : "";
@ -337,20 +343,12 @@ var SidebarUI = {
this._title.value = value;
},
getBroadcasterById(id) {
let sidebarBroadcaster = document.getElementById(id);
if (sidebarBroadcaster && sidebarBroadcaster.localName == "broadcaster") {
return sidebarBroadcaster;
}
return null;
},
/**
* Toggle the visibility of the sidebar. If the sidebar is hidden or is open
* with a different commandID, then the sidebar will be opened using the
* specified commandID. Otherwise the sidebar will be hidden.
*
* @param {string} commandID ID of the xul:broadcaster element to use.
* @param {string} commandID ID of the sidebar.
* @param {DOMNode} [triggerNode] Node, usually a button, that triggered the
* visibility toggling of the sidebar.
* @return {Promise}
@ -363,7 +361,7 @@ var SidebarUI = {
if (!commandID) {
commandID = this._box.getAttribute("sidebarcommand");
}
if (!commandID || !this.getBroadcasterById(commandID)) {
if (!commandID || !this.sidebars.has(commandID)) {
commandID = this.DEFAULT_SIDEBAR_ID;
}
@ -374,28 +372,27 @@ var SidebarUI = {
return this.show(commandID, triggerNode);
},
_loadSidebarExtension(sidebarBroadcaster) {
let extensionId = sidebarBroadcaster.getAttribute("extensionId");
_loadSidebarExtension(commandID) {
let sidebar = this.sidebars.get(commandID);
let {extensionId} = sidebar;
if (extensionId) {
let extensionUrl = sidebarBroadcaster.getAttribute("panel");
let browserStyle = sidebarBroadcaster.getAttribute("browserStyle");
SidebarUI.browser.contentWindow.loadPanel(extensionId, extensionUrl, browserStyle);
SidebarUI.browser.contentWindow.loadPanel(extensionId, sidebar.panel,
sidebar.browserStyle);
}
},
/**
* Show the sidebar, using the parameters from the specified broadcaster.
* @see SidebarUI note.
* Show the sidebar.
*
* This wraps the internal method, including a ping to telemetry.
*
* @param {string} commandID ID of the xul:broadcaster element to use.
* @param {string} commandID ID of the sidebar to use.
* @param {DOMNode} [triggerNode] Node, usually a button, that triggered the
* showing of the sidebar.
*/
show(commandID, triggerNode) {
return this._show(commandID).then((sidebarBroadcaster) => {
this._loadSidebarExtension(sidebarBroadcaster);
return this._show(commandID).then(() => {
this._loadSidebarExtension(commandID);
if (triggerNode) {
updateToggleControlLabel(triggerNode);
@ -410,11 +407,11 @@ var SidebarUI = {
* This is intended to be used when the sidebar is opened automatically
* when a window opens (not triggered by user interaction).
*
* @param {string} commandID ID of the xul:broadcaster element to use.
* @param {string} commandID ID of the sidebar.
*/
showInitially(commandID) {
return this._show(commandID).then((sidebarBroadcaster) => {
this._loadSidebarExtension(sidebarBroadcaster);
return this._show(commandID).then(() => {
this._loadSidebarExtension(commandID);
});
},
@ -422,24 +419,11 @@ var SidebarUI = {
* Implementation for show. Also used internally for sidebars that are shown
* when a window is opened and we don't want to ping telemetry.
*
* @param {string} commandID ID of the xul:broadcaster element to use.
* @param {string} commandID ID of the sidebar.
*/
_show(commandID) {
return new Promise((resolve, reject) => {
let sidebarBroadcaster = this.getBroadcasterById(commandID);
if (!sidebarBroadcaster) {
reject(new Error("Invalid sidebar broadcaster specified: " + commandID));
return;
}
let broadcasters = document.querySelectorAll("broadcaster[group=sidebar]");
for (let broadcaster of broadcasters) {
if (broadcaster != sidebarBroadcaster) {
broadcaster.removeAttribute("checked");
} else {
sidebarBroadcaster.setAttribute("checked", "true");
}
}
return new Promise(resolve => {
this.selectMenuItem(commandID);
this._box.hidden = this._splitter.hidden = false;
this.setPosition();
@ -447,8 +431,8 @@ var SidebarUI = {
this.hideSwitcherPanel();
this._box.setAttribute("checked", "true");
this._box.setAttribute("sidebarcommand", sidebarBroadcaster.id);
this.lastOpenedId = sidebarBroadcaster.id;
this._box.setAttribute("sidebarcommand", commandID);
this.lastOpenedId = commandID;
let {url, title} = this.sidebars.get(commandID);
this.title = title;
@ -459,14 +443,14 @@ var SidebarUI = {
// We're handling the 'load' event before it bubbles up to the usual
// (non-capturing) event handlers. Let it bubble up before resolving.
setTimeout(() => {
resolve(sidebarBroadcaster);
resolve();
// Now that the currentId is updated, fire a show event.
this._fireShowEvent();
}, 0);
}, {capture: true, once: true});
} else {
resolve(sidebarBroadcaster);
resolve();
// Now that the currentId is updated, fire a show event.
this._fireShowEvent();
@ -493,11 +477,7 @@ var SidebarUI = {
this.hideSwitcherPanel();
let commandID = this._box.getAttribute("sidebarcommand");
let sidebarBroadcaster = document.getElementById(commandID);
if (sidebarBroadcaster.getAttribute("checked") != "true") {
return;
}
this.selectMenuItem("");
// Replace the document currently displayed in the sidebar with about:blank
// so that we can free memory by unloading the page. We need to explicitly
@ -507,7 +487,6 @@ var SidebarUI = {
this.browser.setAttribute("src", "about:blank");
this.browser.docShell.createAboutBlankContentViewer(null);
sidebarBroadcaster.removeAttribute("checked");
this._box.removeAttribute("checked");
this._box.hidden = this._splitter.hidden = true;
@ -520,6 +499,24 @@ var SidebarUI = {
updateToggleControlLabel(triggerNode);
}
},
/**
* Sets the checked state only on the menu items of the specified sidebar, or
* none if the argument is an empty string.
*/
selectMenuItem(commandID) {
for (let [id, {menuId, buttonId}] of this.sidebars) {
let menu = document.getElementById(menuId);
let button = document.getElementById(buttonId);
if (id == commandID) {
menu.setAttribute("checked", "true");
button.setAttribute("checked", "true");
} else {
menu.removeAttribute("checked");
button.removeAttribute("checked");
}
}
},
};
// Add getters related to the position here, since we will want them

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

@ -321,28 +321,22 @@ xmlns="http://www.w3.org/1999/xhtml"
orient="vertical"
position="bottomcenter topleft">
<toolbarbutton id="sidebar-switcher-bookmarks"
type="checkbox"
label="&bookmarksButton.label;"
class="subviewbutton subviewbutton-iconic"
key="viewBookmarksSidebarKb"
observes="viewBookmarksSidebar"
oncommand="SidebarUI.show('viewBookmarksSidebar');">
<observes element="viewBookmarksSidebar" attribute="checked"/>
</toolbarbutton>
oncommand="SidebarUI.show('viewBookmarksSidebar');"/>
<toolbarbutton id="sidebar-switcher-history"
type="checkbox"
label="&historyButton.label;"
class="subviewbutton subviewbutton-iconic"
key="key_gotoHistory"
observes="viewHistorySidebar"
oncommand="SidebarUI.show('viewHistorySidebar');">
<observes element="viewHistorySidebar" attribute="checked"/>
</toolbarbutton>
oncommand="SidebarUI.show('viewHistorySidebar');"/>
<toolbarbutton id="sidebar-switcher-tabs"
type="checkbox"
label="&syncedTabs.sidebar.label;"
class="subviewbutton subviewbutton-iconic sync-ui-item"
observes="viewTabsSidebar"
oncommand="SidebarUI.show('viewTabsSidebar');">
<observes element="viewTabsSidebar" attribute="checked"/>
</toolbarbutton>
oncommand="SidebarUI.show('viewTabsSidebar');"/>
<toolbarseparator/>
<!-- Extension toolbarbuttons go here. -->
<toolbarseparator id="sidebar-extensions-separator"/>

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

@ -1,5 +1,6 @@
[DEFAULT]
[browser_sidebar_adopt.js]
[browser_sidebar_keys.js]
[browser_sidebar_move.js]
[browser_sidebar_switcher.js]

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

@ -0,0 +1,24 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
async function testSidebarKeyToggle(key, options, expectedSidebarId) {
let promiseShown = BrowserTestUtils.waitForEvent(window, "SidebarShown");
EventUtils.synthesizeKey(key, options);
await promiseShown;
Assert.equal(document.getElementById("sidebar-box")
.getAttribute("sidebarcommand"), expectedSidebarId);
EventUtils.synthesizeKey(key, options);
Assert.ok(!SidebarUI.isOpen);
}
add_task(async function test_sidebar_keys() {
registerCleanupFunction(() => SidebarUI.hide());
await testSidebarKeyToggle("b", { accelKey: true }, "viewBookmarksSidebar");
if (AppConstants.platform == "win") {
await testSidebarKeyToggle("i", { accelKey: true }, "viewBookmarksSidebar");
}
let options = { accelKey: true, shiftKey: AppConstants.platform == "macosx" };
await testSidebarKeyToggle("h", options, "viewHistorySidebar");
});

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

@ -378,9 +378,7 @@
type="checkbox"
class="subviewbutton subviewbutton-iconic"
key="key_gotoHistory"
oncommand="SidebarUI.toggle('viewHistorySidebar'); PanelUI.hide();">
<observes element="viewHistorySidebar" attribute="checked"/>
</toolbarbutton>
oncommand="SidebarUI.toggle('viewHistorySidebar');"/>
<toolbarbutton id="appMenuClearRecentHistory"
label="&appMenuHistory.clearRecent.label;"
class="subviewbutton subviewbutton-iconic"
@ -425,8 +423,8 @@
<vbox id="PanelUI-remotetabs-buttons">
<toolbarbutton id="PanelUI-remotetabs-view-sidebar"
class="subviewbutton subviewbutton-iconic"
observes="viewTabsSidebar"
label="&appMenuRemoteTabs.sidebar.label;"/>
label="&appMenuRemoteTabs.sidebar.label;"
oncommand="SidebarUI.toggle('viewTabsSidebar');"/>
<toolbarbutton id="PanelUI-remotetabs-view-managedevices"
class="subviewbutton subviewbutton-iconic"
label="&appMenuRemoteTabs.managedevices.label;"

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

@ -116,10 +116,6 @@ this.sidebarAction = class extends ExtensionAPI {
if (button) {
button.remove();
}
let broadcaster = document.getElementById(this.id);
if (broadcaster) {
broadcaster.remove();
}
let header = document.getElementById("sidebar-switcher-target");
header.removeEventListener("SidebarShown", this.updateHeader);
SidebarUI.sidebars.delete(this.id);
@ -145,50 +141,42 @@ this.sidebarAction = class extends ExtensionAPI {
createMenuItem(window, details) {
let {document, SidebarUI} = window;
let keyId = `ext-key-id-${this.id}`;
SidebarUI.sidebars.set(this.id, {
title: details.title,
url: sidebarURL,
menuId: this.menuId,
buttonId: this.buttonId,
// The following properties are specific to extensions
extensionId: this.extension.id,
panel: details.panel,
browserStyle: this.browserStyle,
});
// Use of the broadcaster allows browser-sidebar.js to properly manage the
// checkmarks in the menus.
let broadcaster = document.createXULElement("broadcaster");
broadcaster.setAttribute("id", this.id);
broadcaster.setAttribute("type", "checkbox");
broadcaster.setAttribute("group", "sidebar");
broadcaster.setAttribute("panel", details.panel);
if (this.browserStyle) {
broadcaster.setAttribute("browserStyle", "true");
}
broadcaster.setAttribute("extensionId", this.extension.id);
let id = `ext-key-id-${this.id}`;
broadcaster.setAttribute("key", id);
// oncommand gets attached to menuitem, so we use the observes attribute to
// get the command id we pass to SidebarUI.
broadcaster.setAttribute("oncommand", "SidebarUI.toggle(this.getAttribute('observes'))");
let header = document.getElementById("sidebar-switcher-target");
header.addEventListener("SidebarShown", this.updateHeader);
// Insert a menuitem for View->Show Sidebars.
let menuitem = document.createXULElement("menuitem");
menuitem.setAttribute("id", this.menuId);
menuitem.setAttribute("type", "checkbox");
menuitem.setAttribute("label", details.title);
menuitem.setAttribute("observes", this.id);
menuitem.setAttribute("oncommand", `SidebarUI.toggle("${this.id}");`);
menuitem.setAttribute("class", "menuitem-iconic webextension-menuitem");
menuitem.setAttribute("key", keyId);
this.setMenuIcon(menuitem, details);
// Insert a toolbarbutton for the sidebar dropdown selector.
let toolbarbutton = document.createXULElement("toolbarbutton");
toolbarbutton.setAttribute("id", this.buttonId);
toolbarbutton.setAttribute("type", "checkbox");
toolbarbutton.setAttribute("label", details.title);
toolbarbutton.setAttribute("observes", this.id);
toolbarbutton.setAttribute("oncommand", `SidebarUI.show("${this.id}");`);
toolbarbutton.setAttribute("class", "subviewbutton subviewbutton-iconic webextension-menuitem");
toolbarbutton.setAttribute("key", keyId);
this.setMenuIcon(toolbarbutton, details);
document.getElementById("mainBroadcasterSet").appendChild(broadcaster);
document.getElementById("viewSidebarMenu").appendChild(menuitem);
let separator = document.getElementById("sidebar-extensions-separator");
separator.parentNode.insertBefore(toolbarbutton, separator);
@ -208,8 +196,7 @@ this.sidebarAction = class extends ExtensionAPI {
}
/**
* Update the broadcaster and menuitem `node` with the tab context data
* in `tabData`.
* Update the menu items with the tab context data in `tabData`.
*
* @param {ChromeWindow} window
* Browser chrome window.
@ -224,10 +211,9 @@ this.sidebarAction = class extends ExtensionAPI {
menu = this.createMenuItem(window, tabData);
}
let broadcaster = document.getElementById(this.id);
let urlChanged = tabData.panel !== broadcaster.getAttribute("panel");
let urlChanged = tabData.panel !== SidebarUI.sidebars.get(this.id).panel;
if (urlChanged) {
broadcaster.setAttribute("panel", tabData.panel);
SidebarUI.sidebars.get(this.id).panel = tabData.panel;
}
menu.setAttribute("label", title);
@ -249,7 +235,7 @@ this.sidebarAction = class extends ExtensionAPI {
}
/**
* Update the broadcaster and menuitem for a given window.
* Update the menu items for a given window.
*
* @param {ChromeWindow} window
* Browser chrome window.
@ -260,7 +246,7 @@ this.sidebarAction = class extends ExtensionAPI {
}
/**
* Update the broadcaster and menuitem when the extension changes the icon,
* Update the menu items when the extension changes the icon,
* title, url, etc. If it only changes a parameter for a single tab, `target`
* will be that tab. If it only changes a parameter for a single window,
* `target` will be that window. Otherwise `target` will be null.

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

@ -88,9 +88,6 @@ async function runTests(options) {
sidebarActionId = `${makeWidgetId(extension.id)}-sidebar-action`;
}
let command = document.getElementById(sidebarActionId);
ok(command, "command exists");
let menuId = `menu_${sidebarActionId}`;
let menu = document.getElementById(menuId);
ok(menu, "menu exists");