Bug 1366851 - Flip the sidebar icon depending on if the sidebar is positioned at the start or end of the browser;r=gijs draft
authorBrian Grinstead <bgrinstead@mozilla.com>
Wed, 28 Jun 2017 10:15:48 -0700
changeset 601344 fe7e860f5111deeddec5050ca0a02a69999b4b5e
parent 601255 cc903e3f61894e60c3b0efaf05e9a446d1d85888
child 635216 9ced105ad5ccbca911179a4454375e38f8e51738
push id66015
push userbgrinstead@mozilla.com
push dateWed, 28 Jun 2017 17:16:05 +0000
reviewersgijs
bugs1366851
milestone56.0a1
Bug 1366851 - Flip the sidebar icon depending on if the sidebar is positioned at the start or end of the browser;r=gijs MozReview-Commit-ID: Kaobl1Ox2QZ
browser/base/content/browser-sidebar.js
browser/base/content/test/sidebar/browser_sidebar_move.js
browser/components/customizableui/CustomizableWidgets.jsm
browser/themes/shared/sidebar.inc.css
browser/themes/shared/toolbarbutton-icons.inc.css
--- a/browser/base/content/browser-sidebar.js
+++ b/browser/base/content/browser-sidebar.js
@@ -142,20 +142,20 @@ var SidebarUI = {
     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;
-      // Indicate we've switched ordering to the splitter
-      this._splitter.setAttribute("overlapend", true);
+      // Indicate we've switched ordering to the box
+      this._box.setAttribute("positionend", true);
     } else {
-      this._splitter.removeAttribute("overlapend");
+      this._box.removeAttribute("positionend");
     }
 
     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.
--- a/browser/base/content/test/sidebar/browser_sidebar_move.js
+++ b/browser/base/content/test/sidebar/browser_sidebar_move.js
@@ -28,27 +28,31 @@ function getBrowserChildrenWithOrdinals(
 }
 
 add_task(async function() {
   await SidebarUI.show("viewBookmarksSidebar");
   SidebarUI.showSwitcherPanel();
 
   let reversePositionButton = document.getElementById("sidebar-reverse-position");
   let originalLabel = reversePositionButton.getAttribute("label");
+  let box = document.getElementById("sidebar-box");
 
   // Default (position: left)
   Assert.deepEqual(getBrowserChildrenWithOrdinals(),
     EXPECTED_START_ORDINALS, "Correct ordinal (start)");
+  ok(!box.hasAttribute("positionend"), "Positioned start");
 
   // Moved to right
   SidebarUI.reversePosition();
   SidebarUI.showSwitcherPanel();
   Assert.deepEqual(getBrowserChildrenWithOrdinals(),
     EXPECTED_END_ORDINALS, "Correct ordinal (end)");
   isnot(reversePositionButton.getAttribute("label"), originalLabel, "Label changed");
+  ok(box.hasAttribute("positionend"), "Positioned end");
 
   // Moved to back to left
   SidebarUI.reversePosition();
   SidebarUI.showSwitcherPanel();
   Assert.deepEqual(getBrowserChildrenWithOrdinals(),
     EXPECTED_START_ORDINALS, "Correct ordinal (start)");
+  ok(!box.hasAttribute("positionend"), "Positioned start");
   is(reversePositionButton.getAttribute("label"), originalLabel, "Label is back to normal");
 });
--- a/browser/components/customizableui/CustomizableWidgets.jsm
+++ b/browser/components/customizableui/CustomizableWidgets.jsm
@@ -599,20 +599,25 @@ const CustomizableWidgets = [
     tooltiptext: "sidebar-button.tooltiptext2",
     onCommand(aEvent) {
       let win = aEvent.target.ownerGlobal;
       win.SidebarUI.toggle();
     },
     onCreated(aNode) {
       // Add an observer so the button is checked while the sidebar is open
       let doc = aNode.ownerDocument;
-      let obnode = doc.createElementNS(kNSXUL, "observes");
-      obnode.setAttribute("element", "sidebar-box");
-      obnode.setAttribute("attribute", "checked");
-      aNode.appendChild(obnode);
+      let obChecked = doc.createElementNS(kNSXUL, "observes");
+      obChecked.setAttribute("element", "sidebar-box");
+      obChecked.setAttribute("attribute", "checked");
+      let obPosition = doc.createElementNS(kNSXUL, "observes");
+      obPosition.setAttribute("element", "sidebar-box");
+      obPosition.setAttribute("attribute", "positionend");
+
+      aNode.appendChild(obChecked);
+      aNode.appendChild(obPosition);
     }
   }, {
     id: "social-share-button",
     // custom build our button so we can attach to the share command
     type: "custom",
     onBuild(aDocument) {
       let node = aDocument.createElementNS(kNSXUL, "toolbarbutton");
       node.setAttribute("id", this.id);
--- a/browser/themes/shared/sidebar.inc.css
+++ b/browser/themes/shared/sidebar.inc.css
@@ -20,17 +20,17 @@
   min-width: 1px;
   width: 3px;
   background-image: none !important;
   background-color: transparent;
   margin-inline-start: -3px;
   position: relative;
 }
 
-.sidebar-splitter[overlapend] {
+#sidebar-box[positionend] + .sidebar-splitter {
   border-inline-end-width: 0;
   border-inline-start-width: 1px;
   margin-inline-start: 0;
   margin-inline-end: -3px;
 }
 
 #sidebar-throbber[loading="true"] {
   list-style-image: url("chrome://global/skin/icons/loading.png");
--- a/browser/themes/shared/toolbarbutton-icons.inc.css
+++ b/browser/themes/shared/toolbarbutton-icons.inc.css
@@ -192,16 +192,21 @@ toolbar:not([brighttext]) #bookmarks-men
 #email-link-button[cui-areatype="toolbar"] {
   list-style-image: url("chrome://browser/skin/mail.svg");
 }
 
 #sidebar-button[cui-areatype="toolbar"] {
   list-style-image: url("chrome://browser/skin/sidebars.svg");
 }
 
+#sidebar-button[cui-areatype="toolbar"]:-moz-locale-dir(ltr):not([positionend]),
+#sidebar-button[cui-areatype="toolbar"]:-moz-locale-dir(rtl)[positionend] {
+  transform: scaleX(-1);
+}
+
 #panic-button[cui-areatype="toolbar"] {
   list-style-image: url("chrome://browser/skin/forget.svg");
 }
 
 #panic-button[cui-areatype="toolbar"][open] {
   fill: rgb(213, 32, 20);
 }