Bug 1476030 - Part 2 - Implement xulStore.persist as an alternative to document.persist;r=Gijs draft
authorBrian Grinstead <bgrinstead@mozilla.com>
Wed, 18 Jul 2018 09:43:40 -0700
changeset 819784 227e20b5e3a58c0dc9dc6a87ea2b0894b9b6872e
parent 819783 c65b9395cc192d05d1a348cfbf92f7f59d41dc8f
child 819785 5bd4129d99d7c6d24b8c4d33b1ff00be5e180628
push id116667
push userbgrinstead@mozilla.com
push dateWed, 18 Jul 2018 16:44:07 +0000
reviewersGijs
bugs1476030
milestone63.0a1
Bug 1476030 - Part 2 - Implement xulStore.persist as an alternative to document.persist;r=Gijs MozReview-Commit-ID: CDK4I3189mN
toolkit/components/xulstore/XULStore.js
toolkit/components/xulstore/nsIXULStore.idl
toolkit/components/xulstore/tests/chrome/window_persistence.xul
toolkit/mozapps/extensions/test/browser/browser_bug679604.js
--- a/toolkit/components/xulstore/XULStore.js
+++ b/toolkit/components/xulstore/XULStore.js
@@ -78,18 +78,17 @@ XULStore.prototype = {
   },
 
   /*
    * Internal function for logging debug messages to the Error Console window
    */
   log(message) {
     if (!debugMode)
       return;
-    dump("XULStore: " + message + "\n");
-    Services.console.logStringMessage("XULStore: " + message);
+    console.log("XULStore: " + message);
   },
 
   readFile() {
     try {
       this._data = JSON.parse(Cu.readUTF8File(this._storeFile));
     } catch (e) {
       this.log("Error reading JSON: " + e);
       // This exception could mean that the file didn't exist.
@@ -124,16 +123,40 @@ XULStore.prototype = {
 
     // Don't write the file more than once every 30 seconds.
     this._needsSaving = true;
     this._writeTimer.init(this, WRITE_DELAY_MS, Ci.nsITimer.TYPE_ONE_SHOT);
   },
 
   /* ---------- interface implementation ---------- */
 
+  persist(node, attr) {
+    if (!node.id) {
+      throw new Error("Node without ID passed into persist()");
+    }
+
+    const uri = node.ownerDocument.documentURI;
+    const value = node.getAttribute(attr);
+
+    if (node.localName == "window") {
+      this.log("Persisting attributes to windows is handled by nsXULWindow.");
+      return;
+    }
+
+    // See Bug 1476680 - we could drop the `hasValue` check so that
+    // any time there's an empty attribute it gets removed from the
+    // store. Since this is copying behavior from document.persist,
+    // callers would need to be updated with that change.
+    if (!value && this.hasValue(uri, node.id, attr)) {
+      this.removeValue(uri, node.id, attr);
+    } else {
+      this.setValue(uri, node.id, attr, value);
+    }
+  },
+
   setValue(docURI, id, attr, value) {
     this.log("Saving " + attr + "=" + value + " for id=" + id + ", doc=" + docURI);
 
     if (!this._saveAllowed) {
       Services.console.logStringMessage("XULStore: Changes after profile-before-change are ignored!");
       return;
     }
 
--- a/toolkit/components/xulstore/nsIXULStore.idl
+++ b/toolkit/components/xulstore/nsIXULStore.idl
@@ -1,28 +1,41 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 
 interface nsIStringEnumerator;
+webidl Node;
 
 /**
  * The XUL store is used to store information related to a XUL document/application.
  * Typically it is used to store the persisted state for the document, such as
  * window location, toolbars that are open and nodes that are open and closed in a tree.
  *
  * The data is serialized to [profile directory]/xulstore.json
  */
 [scriptable, uuid(987c4b35-c426-4dd7-ad49-3c9fa4c65d20)]
 
 interface nsIXULStore: nsISupports
 {
   /**
+   * Sets a value for a specified node's attribute, except in
+   * the case below (following the original XULDocument::persist):
+   * If the value is empty and if calling `hasValue` with the node's
+   * document and ID and `attr` would return true, then the
+   * value instead gets removed from the store (see Bug 1476680).
+   *
+   * @param node - DOM node
+   * @param attr - attribute to store
+   */
+  void persist(in Node aNode, in AString attr);
+
+  /**
    * Sets a value in the store.
    *
    * @param doc - document URI
    * @param id - identifier of the node
    * @param attr - attribute to store
    * @param value - value of the attribute
    */
   void setValue(in AString doc, in AString id, in AString attr, in AString value);
--- a/toolkit/components/xulstore/tests/chrome/window_persistence.xul
+++ b/toolkit/components/xulstore/tests/chrome/window_persistence.xul
@@ -19,40 +19,42 @@ let URI = "chrome://mochitests/content/c
 function opened()
 {
   runTest();
 }
 
 function runTest()
 {
   var firstRun = window.arguments[0];
+  var button1 = document.getElementById("button1");
+  var button2 = document.getElementById("button2");
   if (firstRun) {
-    document.getElementById("button1").setAttribute("value", "Pressed");
-    document.getElementById("button2").removeAttribute("value");
+    button1.setAttribute("value", "Pressed");
+    button2.removeAttribute("value");
 
-    document.getElementById("button2").setAttribute("foo", "bar");
-    document.persist("button2", "foo");
-    is(XULStore.getValue(URI, "button2", "foo"), "bar", "attribute persisted")
-    document.getElementById("button2").removeAttribute("foo");
-    document.persist("button2", "foo");
-    is(XULStore.hasValue(URI, "button2", "foo"), false, "attribute removed")
+    button2.setAttribute("foo", "bar");
+    XULStore.persist(button2, "foo");
+    is(XULStore.getValue(URI, "button2", "foo"), "bar", "attribute persisted");
+    button2.removeAttribute("foo");
+    XULStore.persist(button2, "foo");
+    is(XULStore.hasValue(URI, "button2", "foo"), false, "attribute removed");
 
     window.close();
     window.opener.windowOpened();
   }
   else {
-    is(document.getElementById("button1").getAttribute("value"), "Pressed",
+    is(button1.getAttribute("value"), "Pressed",
        "Attribute set");
-    is(document.getElementById("button2").hasAttribute("value"), true,
+    is(button2.hasAttribute("value"), true,
        "Attribute cleared");
-    is(document.getElementById("button2").getAttribute("value"), "",
+    is(button2.getAttribute("value"), "",
        "Attribute cleared");
-    is(document.getElementById("button2").hasAttribute("foo"), false,
+    is(button2.hasAttribute("foo"), false,
        "Attribute cleared");
-    is(document.getElementById("button2").getAttribute("foo"), "",
+    is(button2.getAttribute("foo"), "",
        "Attribute cleared");
 
     window.close();
     window.opener.testDone();
   }
 }
 
 function is(l, r, n) { window.opener.wrappedJSObject.SimpleTest.is(l,r,n); }
--- a/toolkit/mozapps/extensions/test/browser/browser_bug679604.js
+++ b/toolkit/mozapps/extensions/test/browser/browser_bug679604.js
@@ -6,17 +6,17 @@
 // Firefox doesn't break the add-ons manager when that category doesn't exist
 
 async function test() {
   waitForExplicitFinish();
 
   let aWindow = await open_manager(null);
   var categories = aWindow.document.getElementById("categories");
   categories.setAttribute("last-selected", "foo");
-  aWindow.document.persist("categories", "last-selected");
+  Services.xulStore.persist(categories, "last-selected");
 
   await close_manager(aWindow);
   Services.prefs.clearUserPref(PREF_UI_LASTCATEGORY);
 
   aWindow = await open_manager(null);
   is(new CategoryUtilities(aWindow).selectedCategory, "discover",
      "Should have loaded the right view");