Bug 1355331 - Create an option to move sidebar between the left and right sides of the window;r=mikedeboer
MozReview-Commit-ID: 79ts9djMC3e
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1267,16 +1267,19 @@ pref("pdfjs.firstRun", true);
pref("pdfjs.previousHandler.preferredAction", 0);
pref("pdfjs.previousHandler.alwaysAskBeforeHandling", false);
// The maximum amount of decoded image data we'll willingly keep around (we
// might keep around more than this, but we'll try to get down to this value).
// (This is intentionally on the high side; see bug 746055.)
pref("image.mem.max_decoded_image_kb", 256000);
+// Is the sidebar positioned ahead of the content browser
+pref("sidebar.position_start", true);
+
// Activation from inside of share panel is possible if activationPanelEnabled
// is true. Pref'd off for release while usage testing is done through beta.
pref("social.share.activationPanelEnabled", true);
pref("social.shareDirectory", "https://activations.cdn.mozilla.net/sharePanel.html");
// Block insecure active content on https pages
pref("security.mixed_content.block_active_content", true);
--- a/browser/base/content/browser-sidebar.js
+++ b/browser/base/content/browser-sidebar.js
@@ -16,27 +16,34 @@
* display on the sidebar.
* - checked indicates whether the sidebar is currently displayed.
* Note that toggleSidebar updates this attribute when
* it changes the sidebar's visibility.
* - group this attribute must be set to "sidebar".
*/
var SidebarUI = {
browser: null,
+ POSITION_START_PREF: "sidebar.position_start",
_box: null,
_title: null,
_splitter: null,
+ _icon: null,
+ _reversePositionButton: null,
+ _switcherPanel: null,
+ _switcherTarget: null,
+ _switcherArrow: null,
init() {
this._box = document.getElementById("sidebar-box");
this.browser = document.getElementById("sidebar");
this._title = document.getElementById("sidebar-title");
this._splitter = document.getElementById("sidebar-splitter");
this._icon = document.getElementById("sidebar-icon");
+ this._reversePositionButton = document.getElementById("sidebar-reverse-position");
this._switcherPanel = document.getElementById("sidebarMenu-popup");
this._switcherTarget = document.getElementById("sidebar-switcher-target");
this._switcherArrow = document.getElementById("sidebar-switcher-arrow");
this._switcherTarget.addEventListener("command", () => {
this.toggleSwitcherPanel();
});
},
@@ -67,16 +74,23 @@ var SidebarUI = {
this._switcherPanel.hidePopup();
},
showSwitcherPanel() {
this._ensureShortcutsShown();
this._switcherPanel.addEventListener("popuphiding", () => {
this._switcherTarget.classList.remove("active");
}, {once: true});
+
+ // Combine start/end position with ltr/rtl to set the label in the popup appropriately.
+ let label = this._positionStart == this.isRTL ?
+ gNavigatorBundle.getString("sidebar.moveToLeft") :
+ gNavigatorBundle.getString("sidebar.moveToRight");
+ this._reversePositionButton.setAttribute("label", label);
+
this._switcherPanel.hidden = false;
this._switcherPanel.openPopup(this._icon);
this._switcherTarget.classList.add("active");
},
_addedShortcuts: false,
_ensureShortcutsShown() {
if (this._addedShortcuts) {
@@ -89,16 +103,47 @@ var SidebarUI = {
if (!key) {
continue;
}
button.setAttribute("shortcut", ShortcutUtils.prettifyShortcut(key));
}
},
/**
+ * Change the pref that will trigger a call to setPosition
+ */
+ reversePosition() {
+ Services.prefs.setBoolPref(this.POSITION_START_PREF, !this._positionStart);
+ },
+
+ /**
+ * Read the positioning pref and position the sidebar and the splitter
+ * appropriately within the browser container.
+ */
+ setPosition() {
+ // First reset all ordinals to match DOM ordering.
+ let browser = document.getElementById("browser");
+ [...browser.childNodes].forEach((node, i) => {
+ node.ordinal = i + 1;
+ });
+
+ if (!this._positionStart) {
+ // DOM ordering is: | sidebar-box | splitter | appcontent |
+ // Want to display as: | appcontent | splitter | sidebar-box |
+ // So we just swap box and appcontent ordering
+ let appcontent = document.getElementById("appcontent");
+ let boxOrdinal = this._box.ordinal;
+ this._box.ordinal = appcontent.ordinal;
+ appcontent.ordinal = boxOrdinal;
+ }
+
+ this.hideSwitcherPanel();
+ },
+
+ /**
* Try and adopt the status of the sidebar from another window.
* @param {Window} sourceWindow - Window to use as a source for sidebar status.
* @return true if we adopted the state, or false if the caller should
* initialize the state itself.
*/
adoptFromWindow(sourceWindow) {
// If the opener had a sidebar, open the same sidebar in our window.
// The opener can be the hidden window too, if we're coming from the state
@@ -127,18 +172,17 @@ var SidebarUI = {
this._box.setAttribute("width", sourceUI._box.boxObject.width);
this._box.setAttribute("sidebarcommand", commandID);
// Note: we're setting 'src' on this._box, which is a <vbox>, not on
// the <browser id="sidebar">. This lets us delay the actual load until
// delayedStartup().
this._box.setAttribute("src", sourceUI.browser.getAttribute("src"));
- this._box.hidden = false;
- this._splitter.hidden = false;
+ this._setVisibility(true);
commandElem.setAttribute("checked", "true");
this.browser.setAttribute("src", this._box.getAttribute("src"));
return true;
},
windowPrivacyMatches(w1, w2) {
return PrivateBrowsingUtils.isWindowPrivate(w1) === PrivateBrowsingUtils.isWindowPrivate(w2);
},
@@ -165,18 +209,17 @@ var SidebarUI = {
// If we're not adopting settings from a parent window, set them now.
let commandID = this._box.getAttribute("sidebarcommand");
if (!commandID) {
return;
}
let command = document.getElementById(commandID);
if (command) {
- this._box.hidden = false;
- this._splitter.hidden = false;
+ this._setVisibility(true);
command.setAttribute("checked", "true");
this.browser.setAttribute("src", this._box.getAttribute("src"));
} else {
// Remove the |sidebarcommand| attribute, because the element it
// refers to no longer exists, so we should assume this sidebar
// panel has been uninstalled. (249883)
this._box.removeAttribute("sidebarcommand");
}
@@ -215,16 +258,29 @@ var SidebarUI = {
return this._title.value;
},
set title(value) {
this._title.value = value;
},
/**
+ * Internal helper to show/hide the box and splitter elements.
+ *
+ * @param {bool} visible
+ */
+ _setVisibility(visible) {
+ this._box.hidden = !visible;
+ this._splitter.hidden = !visible;
+ if (visible) {
+ this.setPosition();
+ }
+ },
+
+ /**
* 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.
* @return {Promise}
*/
toggle(commandID = this.currentID) {
@@ -263,18 +319,17 @@ var SidebarUI = {
if (broadcaster != sidebarBroadcaster) {
broadcaster.removeAttribute("checked");
} else {
sidebarBroadcaster.setAttribute("checked", "true");
}
}
- this._box.hidden = false;
- this._splitter.hidden = false;
+ this._setVisibility(true);
this.hideSwitcherPanel();
this._box.setAttribute("sidebarcommand", sidebarBroadcaster.id);
this.lastOpenedId = sidebarBroadcaster.id;
let title = sidebarBroadcaster.getAttribute("sidebartitle");
if (!title) {
@@ -342,28 +397,35 @@ var SidebarUI = {
// until about:blank has loaded (which does not happen as long as the
// element is hidden).
this.browser.setAttribute("src", "about:blank");
this.browser.docShell.createAboutBlankContentViewer(null);
sidebarBroadcaster.removeAttribute("checked");
this._box.setAttribute("sidebarcommand", "");
this._title.value = "";
- this._box.hidden = true;
- this._splitter.hidden = true;
+ this._setVisibility(false);
let selBrowser = gBrowser.selectedBrowser;
selBrowser.focus();
selBrowser.messageManager.sendAsyncMessage("Sidebar:VisibilityChange",
{commandID, isOpen: false}
);
BrowserUITelemetry.countSidebarEvent(commandID, "hide");
},
};
+// Add getters related to the position here, since we will want them
+// available for both startDelayedLoad and init.
+XPCOMUtils.defineLazyPreferenceGetter(SidebarUI, "_positionStart",
+ SidebarUI.POSITION_START_PREF, true, SidebarUI.setPosition.bind(SidebarUI));
+XPCOMUtils.defineLazyGetter(SidebarUI, "isRTL", () => {
+ return getComputedStyle(document.documentElement).direction == "rtl";
+});
+
/**
* This exists for backwards compatibility - it will be called once a sidebar is
* ready, following any request to show it.
*
* @deprecated
*/
function fireSidebarFocusedEvent() {}
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -316,16 +316,20 @@
<toolbarbutton id="sidebar-switcher-tabs"
label="&syncedTabs.sidebar.label;"
class="subviewbutton subviewbutton-iconic"
observes="viewTabsSidebar"
oncommand="SidebarUI.show('viewTabsSidebar');">
<observes element="viewTabsSidebar" attribute="checked"/>
</toolbarbutton>
<toolbarseparator/>
+ <toolbarbutton id="sidebar-reverse-position"
+ class="subviewbutton"
+ oncommand="SidebarUI.reversePosition()"/>
+ <toolbarseparator/>
<toolbarbutton label="&sidebarMenuClose.label;"
class="subviewbutton"
oncommand="SidebarUI.hide()"/>
</panel>
<menupopup id="toolbar-context-menu"
onpopupshowing="onViewToolbarsPopupShowing(event, document.getElementById('viewToolbarsMenuSeparator'));">
<menuitem oncommand="gCustomizeMode.addToPanel(document.popupNode)"
--- a/browser/base/content/test/sidebar/browser.ini
+++ b/browser/base/content/test/sidebar/browser.ini
@@ -1,4 +1,5 @@
[DEFAULT]
[browser_bug409481.js]
+[browser_sidebar_move.js]
[browser_sidebar_switcher.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/sidebar/browser_sidebar_move.js
@@ -0,0 +1,54 @@
+
+registerCleanupFunction(() => {
+ Services.prefs.clearUserPref("sidebar.position_start");
+ SidebarUI.hide();
+});
+
+const EXPECTED_START_ORDINALS = [
+ ["browser-border-start", 1],
+ ["sidebar-box", 2],
+ ["sidebar-splitter", 3],
+ ["appcontent", 4],
+ ["browser-border-end", 5],
+];
+
+const EXPECTED_END_ORDINALS = [
+ ["browser-border-start", 1],
+ ["sidebar-box", 4],
+ ["sidebar-splitter", 3],
+ ["appcontent", 2],
+ ["browser-border-end", 5],
+];
+
+function getBrowserChildrenWithOrdinals() {
+ let browser = document.getElementById("browser");
+ return [...browser.childNodes].map(node => {
+ return [node.id, node.ordinal];
+ });
+}
+
+add_task(function* () {
+ yield SidebarUI.show("viewBookmarksSidebar");
+ SidebarUI.showSwitcherPanel();
+
+ let reversePositionButton = document.getElementById("sidebar-reverse-position");
+ let originalLabel = reversePositionButton.getAttribute("label");
+
+ // Default (position: left)
+ Assert.deepEqual(getBrowserChildrenWithOrdinals(),
+ EXPECTED_START_ORDINALS, "Correct ordinal (start)");
+
+ // Moved to right
+ SidebarUI.reversePosition();
+ SidebarUI.showSwitcherPanel();
+ Assert.deepEqual(getBrowserChildrenWithOrdinals(),
+ EXPECTED_END_ORDINALS, "Correct ordinal (end)");
+ isnot(reversePositionButton.getAttribute("label"), originalLabel, "Label changed");
+
+ // Moved to back to left
+ SidebarUI.reversePosition();
+ SidebarUI.showSwitcherPanel();
+ Assert.deepEqual(getBrowserChildrenWithOrdinals(),
+ EXPECTED_START_ORDINALS, "Correct ordinal (start)");
+ is(reversePositionButton.getAttribute("label"), originalLabel, "Label is back to normal");
+});
--- a/browser/base/content/test/sidebar/browser_sidebar_switcher.js
+++ b/browser/base/content/test/sidebar/browser_sidebar_switcher.js
@@ -1,8 +1,12 @@
+
+registerCleanupFunction(() => {
+ SidebarUI.hide();
+});
function showSwitcherPanelPromise() {
return new Promise(resolve => {
SidebarUI._switcherPanel.addEventListener("popupshown", () => {
resolve();
}, {once: true});
SidebarUI.showSwitcherPanel();
});
--- a/browser/locales/en-US/chrome/browser/browser.properties
+++ b/browser/locales/en-US/chrome/browser/browser.properties
@@ -585,16 +585,20 @@ fullscreenButton.tooltip=Display the win
service.toolbarbutton.label=Services
service.toolbarbutton.tooltiptext=Services
# LOCALIZATION NOTE (social.install.description): %1$S is the hostname of the social provider, %2$S is brandShortName (e.g. Firefox)
service.install.description=Would you like to enable services from %1$S to display in your %2$S toolbar and sidebar?
service.install.ok.label=Enable Services
service.install.ok.accesskey=E
+# These are visible when opening the popup inside the bookmarks sidebar
+sidebar.moveToLeft=Move sidebar to left
+sidebar.moveToRight=Move sidebar to right
+
# LOCALIZATION NOTE (social.markpageMenu.label): %S is the name of the social provider
social.markpageMenu.label=Save Page to %S
# LOCALIZATION NOTE (social.marklinkMenu.label): %S is the name of the social provider
social.marklinkMenu.label=Save Link to %S
# LOCALIZATION NOTE (social.error.message): %1$S is brandShortName (e.g. Firefox), %2$S is the name of the social provider
social.error.message=%1$S is unable to connect with %2$S right now.
social.error.tryAgain.label=Try Again