Bug 907079 - Don't add unnecessary history state for reader mode style dropdown draft
authorMargaret Leibovic <margaret.leibovic@gmail.com>
Wed, 28 Jan 2015 17:43:07 +0100
changeset 239452 4194bc12d4c52c6b2b7749a6cc4190d2019a7224
parent 239451 7ddcf91ad7741521f87f10b4e7dfdb003749d23c
child 505172 9526ed112cb8425a8253da23a87e9e17fbb9598f
push id497
push usermleibovic@mozilla.com
push dateWed, 28 Jan 2015 16:43:37 +0000
bugs907079
milestone38.0a1
Bug 907079 - Don't add unnecessary history state for reader mode style dropdown
toolkit/components/reader/AboutReader.jsm
toolkit/components/reader/content/aboutReader.html
--- a/toolkit/components/reader/AboutReader.jsm
+++ b/toolkit/components/reader/AboutReader.jsm
@@ -51,17 +51,17 @@ let AboutReader = function(mm, win) {
 
   win.addEventListener("unload", this, false);
   win.addEventListener("scroll", this, false);
   win.addEventListener("popstate", this, false);
   win.addEventListener("resize", this, false);
 
   doc.addEventListener("visibilitychange", this, false);
 
-  this._setupAllDropdowns();
+  this._setupStyleDropdown();
   this._setupButton("toggle-button", this._onReaderToggle.bind(this));
   this._setupButton("share-button", this._onShare.bind(this));
 
   let colorSchemeOptions = [
     { name: gStrings.GetStringFromName("aboutReader.colorSchemeDark"),
       value: "dark"},
     { name: gStrings.GetStringFromName("aboutReader.colorSchemeLight"),
       value: "light"},
@@ -197,20 +197,30 @@ AboutReader.prototype = {
         // XXX: Don't toggle the toolbar on double click. (See the "Gesture:DoubleTap" handler in Reader.js)
         this._toggleToolbarVisibility();
         break;
       case "scroll":
         let isScrollingUp = this._scrollOffset > aEvent.pageY;
         this._setToolbarVisibility(isScrollingUp);
         this._scrollOffset = aEvent.pageY;
         break;
-      case "popstate":
-        if (!aEvent.state)
-          this._closeAllDropdowns();
+      case "popstate": {
+        if (aEvent.state) {
+          if (aEvent.state.dropdown === 1) {
+            this._openStyleDropdown();
+          } else {
+            this._closeStyleDropdown();
+          }
+        } else {
+          // We need to account for the original history state we added to keep
+          // track of whether or not the dropdown is shown.
+          this._win.history.back();
+        }
         break;
+      }
       case "resize":
         this._updateImageMargins();
         break;
 
       case "devicelight":
         this._handleDeviceLight(aEvent.value);
         break;
 
@@ -401,18 +411,22 @@ AboutReader.prototype = {
   },
 
   _getToolbarVisibility: function Reader_getToolbarVisibility() {
     return !this._toolbarElement.classList.contains("toolbar-hidden");
   },
 
   _setToolbarVisibility: function Reader_setToolbarVisibility(visible) {
     let win = this._win;
-    if (win.history.state)
-      win.history.back();
+
+    // Remove any history state we created for the dropdown.
+    if (win.history.state) {
+      this._closeStyleDropdown();
+      win.history.replaceState({ dropdown: 0 }, "");
+    }
 
     if (!this._toolbarEnabled)
       return;
 
     // Don't allow visible toolbar until banner state is known
     if (this._isReadingListItem == -1)
       return;
 
@@ -691,102 +705,77 @@ AboutReader.prototype = {
       if (!aEvent.isTrusted)
         return;
 
       aEvent.stopPropagation();
       callback();
     }, true);
   },
 
-  _setupAllDropdowns: function Reader_setupAllDropdowns() {
+  _setupStyleDropdown: function Reader_setupStyleDropdown() {
     let doc = this._doc;
     let win = this._win;
 
-    let dropdowns = doc.getElementsByClassName("dropdown");
-
-    for (let i = dropdowns.length - 1; i >= 0; i--) {
-      let dropdown = dropdowns[i];
+    // Use history state to keep track of whether or not the dropdown is open,
+    // so that the user can use the back button to close it.
+    win.history.pushState({ dropdown: 0 }, "");
 
-      let dropdownToggle = dropdown.getElementsByClassName("dropdown-toggle")[0];
-      let dropdownPopup = dropdown.getElementsByClassName("dropdown-popup")[0];
+    let dropdown = doc.getElementById("style-dropdown");
 
-      if (!dropdownToggle || !dropdownPopup)
-        continue;
-
-      let dropdownArrow = doc.createElement("div");
-      dropdownArrow.className = "dropdown-arrow";
-      dropdownPopup.appendChild(dropdownArrow);
+    let dropdownToggle = dropdown.querySelector(".dropdown-toggle");
+    let dropdownPopup = dropdown.querySelector(".dropdown-popup");
+    let dropdownArrow = dropdown.querySelector(".dropdown-arrow");
 
-      let updatePopupPosition = function() {
-        let popupWidth = dropdownPopup.offsetWidth + 30;
-        let arrowWidth = dropdownArrow.offsetWidth;
-        let toggleWidth = dropdownToggle.offsetWidth;
-        let toggleLeft = dropdownToggle.offsetLeft;
+    let updatePopupPosition = function() {
+      let popupWidth = dropdownPopup.offsetWidth + 30;
+      let arrowWidth = dropdownArrow.offsetWidth;
+      let toggleWidth = dropdownToggle.offsetWidth;
+      let toggleLeft = dropdownToggle.offsetLeft;
 
-        let popupShift = (toggleWidth - popupWidth) / 2;
-        let popupLeft = Math.max(0, Math.min(win.innerWidth - popupWidth, toggleLeft + popupShift));
-        dropdownPopup.style.left = popupLeft + "px";
+      let popupShift = (toggleWidth - popupWidth) / 2;
+      let popupLeft = Math.max(0, Math.min(win.innerWidth - popupWidth, toggleLeft + popupShift));
+      dropdownPopup.style.left = popupLeft + "px";
 
-        let arrowShift = (toggleWidth - arrowWidth) / 2;
-        let arrowLeft = toggleLeft - popupLeft + arrowShift;
-        dropdownArrow.style.left = arrowLeft + "px";
-      };
+      let arrowShift = (toggleWidth - arrowWidth) / 2;
+      let arrowLeft = toggleLeft - popupLeft + arrowShift;
+      dropdownArrow.style.left = arrowLeft + "px";
+    };
 
-      win.addEventListener("resize", function(aEvent) {
-        if (!aEvent.isTrusted)
-          return;
+    win.addEventListener("resize", event => {
+      if (!event.isTrusted)
+        return;
 
-        // Wait for reflow before calculating the new position of the popup.
-        win.setTimeout(updatePopupPosition, 0);
-      }, true);
+      // Wait for reflow before calculating the new position of the popup.
+      win.setTimeout(updatePopupPosition, 0);
+    }, true);
 
-      dropdownToggle.addEventListener("click", function(aEvent) {
-        if (!aEvent.isTrusted)
-          return;
+    dropdownToggle.addEventListener("click", event => {
+      if (!event.isTrusted)
+        return;
 
-        aEvent.stopPropagation();
+      event.stopPropagation();
 
-        if (!this._getToolbarVisibility())
-          return;
-
-        let dropdownClasses = dropdown.classList;
+      if (!this._getToolbarVisibility())
+        return;
 
-        if (dropdownClasses.contains("open")) {
-          win.history.back();
-        } else {
-          updatePopupPosition();
-          if (!this._closeAllDropdowns())
-            this._pushDropdownState();
+      if (dropdown.classList.contains("open")) {
+        this._closeStyleDropdown();
+        win.history.replaceState({ dropdown: 0 }, "");
+      } else {
+        updatePopupPosition();
+        this._openStyleDropdown();
 
-          dropdownClasses.add("open");
-        }
-      }.bind(this), true);
-    }
+        // Use replaceState to avoid adding more session history entries.
+        win.history.replaceState({ dropdown: 1 }, "");
+      }
+    }, true);
   },
 
-  _pushDropdownState: function Reader_pushDropdownState() {
-    // FIXME: We're getting a NS_ERROR_UNEXPECTED error when we try
-    // to do win.history.pushState() here (see bug 682296). This is
-    // a workaround that allows us to push history state on the target
-    // content document.
-
-    let doc = this._doc;
-    let body = doc.body;
-
-    if (this._pushStateScript)
-      body.removeChild(this._pushStateScript);
-
-    this._pushStateScript = doc.createElement('script');
-    this._pushStateScript.type = "text/javascript";
-    this._pushStateScript.innerHTML = 'history.pushState({ dropdown: 1 }, document.title);';
-
-    body.appendChild(this._pushStateScript);
+  _closeStyleDropdown: function() {
+    let dropdown = this._doc.getElementById("style-dropdown");
+    dropdown.classList.remove("open");
   },
 
-  _closeAllDropdowns : function Reader_closeAllDropdowns() {
-    let dropdowns = this._doc.querySelectorAll(".dropdown.open");
-    for (let i = dropdowns.length - 1; i >= 0; i--) {
-      dropdowns[i].classList.remove("open");
-    }
-
-    return (dropdowns.length > 0)
+  _openStyleDropdown: function() {
+    let dropdown = this._doc.getElementById("style-dropdown");
+    dropdown.classList.add("open");
   }
 };
--- a/toolkit/components/reader/content/aboutReader.html
+++ b/toolkit/components/reader/content/aboutReader.html
@@ -21,24 +21,25 @@
   <div id="reader-content" class="content">
   </div>
 
   <div id="reader-message" class="message">
   </div>
 
   <ul id="reader-toolbar" class="toolbar toolbar-hidden">
     <li><a id="share-button" class="button share-button" href="#"></a></li>
-    <ul class="dropdown">
+    <ul id="style-dropdown" class="dropdown">
       <li><a class="dropdown-toggle button style-button" href="#"></a></li>
       <li class="dropdown-popup">
         <ul id="font-type-buttons"></ul>
         <hr></hr>
         <ul id="font-size-buttons" class="segmented-button"></ul>
         <hr></hr>
         <ul id="color-scheme-buttons" class="segmented-button"></ul>
+        <div class="dropdown-arrow"/>
       </li>
     </ul>
     <li><a id="toggle-button" class="button toggle-button" href="#"></a></li>
   </ul>
 
 </body>
 
 </html>