Bug 1501410 - Part 2 - Add support for recovering deleted preferences. r=bgrins
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Tue, 15 Jan 2019 23:48:11 +0000
changeset 511135 5b558b35ac4fc7727976d60a6f9da945081ac230
parent 511134 20f2bbf59893ed5e92777f81c341b15a546059e2
child 511136 2039967a0b3b7f98bf7ffc70047a99cf85d08860
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1501410
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1501410 - Part 2 - Add support for recovering deleted preferences. r=bgrins This updates the user interface for adding new preferences to be closer to the mockup, and makes deleted preferences more similar to rows that are not added yet. This will later make it simpler to react to external changes in the underlying data. Differential Revision: https://phabricator.services.mozilla.com/D15945
browser/components/aboutconfig/content/aboutconfig.css
browser/components/aboutconfig/content/aboutconfig.js
browser/components/aboutconfig/content/aboutconfig.notftl
browser/components/aboutconfig/test/browser/browser_edit.js
browser/components/aboutconfig/test/browser/head.js
--- a/browser/components/aboutconfig/content/aboutconfig.css
+++ b/browser/components/aboutconfig/content/aboutconfig.css
@@ -58,17 +58,23 @@
   padding: 4px;
   width: 50%;
 }
 
 #prefs > tr > td.cell-name {
   padding-inline-start: 30px;
 }
 
+#prefs > tr.deleted > td.cell-name {
+  font-weight: bold;
+  opacity: 0.4;
+}
+
 .cell-value {
   word-break: break-all;
 }
 
-td.cell-value > form > input {
+td.cell-value > form > input[type="text"],
+td.cell-value > form > input[type="number"] {
   -moz-appearance: textfield;
   width: 100%;
   box-sizing: border-box;
 }
--- a/browser/components/aboutconfig/content/aboutconfig.js
+++ b/browser/components/aboutconfig/content/aboutconfig.js
@@ -22,46 +22,59 @@ let gElementToPrefMap = new WeakMap();
 /**
  * Reference to the PrefRow currently being edited, if any.
  */
 let gPrefInEdit = null;
 
 class PrefRow {
   constructor(name) {
     this.name = name;
+    this.value = true;
     this.refreshValue();
 
     this.editing = false;
     this.element = document.createElement("tr");
     this._setupElement();
     gElementToPrefMap.set(this.element, this);
   }
 
   refreshValue() {
+    this.hasDefaultValue = prefHasDefaultValue(this.name);
     this.hasUserValue = Services.prefs.prefHasUserValue(this.name);
-    this.hasDefaultValue = this.hasUserValue ? prefHasDefaultValue(this.name)
-                                             : true;
     this.isLocked = Services.prefs.prefIsLocked(this.name);
 
+    // If this preference has been deleted, we keep its last known value.
+    if (!this.exists) {
+      return;
+    }
+
     try {
       // This can throw for locked preferences without a default value.
       this.value = Preferences.get(this.name);
       // We don't know which preferences should be read using getComplexValue,
       // so we use a heuristic to determine if this is a localized preference.
       if (!this.hasUserValue &&
           /^chrome:\/\/.+\/locale\/.+\.properties/.test(this.value)) {
         // This can throw if there is no value in the localized files.
         this.value = Services.prefs.getComplexValue(this.name,
           Ci.nsIPrefLocalizedString).data;
       }
     } catch (ex) {
       this.value = "";
     }
   }
 
+  get type() {
+    return this.value.constructor.name;
+  }
+
+  get exists() {
+    return this.hasDefaultValue || this.hasUserValue;
+  }
+
   _setupElement() {
     this.element.textContent = "";
     let nameCell = document.createElement("td");
     this.element.append(
       nameCell,
       this.valueCell = document.createElement("td"),
       this.editCell = document.createElement("td"),
       this.resetCell = document.createElement("td")
@@ -83,48 +96,81 @@ class PrefRow {
     nameCell.append(parts[parts.length - 1]);
 
     this.refreshElement();
   }
 
   refreshElement() {
     this.element.classList.toggle("has-user-value", !!this.hasUserValue);
     this.element.classList.toggle("locked", !!this.isLocked);
-    if (!this.editing) {
+    this.element.classList.toggle("deleted", !this.exists);
+    if (this.exists && !this.editing) {
       this.valueCell.textContent = this.value;
-      if (this.value.constructor.name == "Boolean") {
+      if (this.type == "Boolean") {
         document.l10n.setAttributes(this.editButton, "about-config-pref-toggle");
         this.editButton.className = "button-toggle";
       } else {
         document.l10n.setAttributes(this.editButton, "about-config-pref-edit");
         this.editButton.className = "button-edit";
       }
       this.editButton.removeAttribute("form");
       delete this.inputField;
     } else {
       this.valueCell.textContent = "";
       // The form is needed for the validation report to appear, but we need to
       // prevent the associated button from reloading the page.
       let form = document.createElement("form");
       form.addEventListener("submit", event => event.preventDefault());
       form.id = "form-edit";
-      this.inputField = document.createElement("input");
-      this.inputField.value = this.value;
-      if (this.value.constructor.name == "Number") {
-        this.inputField.type = "number";
-        this.inputField.required = true;
-        this.inputField.min = -2147483648;
-        this.inputField.max = 2147483647;
+      if (this.editing) {
+        this.inputField = document.createElement("input");
+        this.inputField.value = this.value;
+        if (this.type == "Number") {
+          this.inputField.type = "number";
+          this.inputField.required = true;
+          this.inputField.min = -2147483648;
+          this.inputField.max = 2147483647;
+        } else {
+          this.inputField.type = "text";
+        }
+        form.appendChild(this.inputField);
+        document.l10n.setAttributes(this.editButton, "about-config-pref-save");
+        this.editButton.className = "primary button-save";
       } else {
-        this.inputField.type = "text";
+        delete this.inputField;
+        for (let type of ["Boolean", "Number", "String"]) {
+          let radio = document.createElement("input");
+          radio.type = "radio";
+          radio.name = "type";
+          radio.value = type;
+          radio.checked = this.type == type;
+          form.appendChild(radio);
+          let radioLabel = document.createElement("span");
+          radioLabel.textContent = type;
+          form.appendChild(radioLabel);
+        }
+        form.addEventListener("click", event => {
+          if (event.target.name != "type") {
+            return;
+          }
+          let type = event.target.value;
+          if (this.type != type) {
+            if (type == "Boolean") {
+              this.value = true;
+            } else if (type == "Number") {
+              this.value = 0;
+            } else {
+              this.value = "";
+            }
+          }
+        });
+        document.l10n.setAttributes(this.editButton, "about-config-pref-add");
+        this.editButton.className = "button-add";
       }
-      form.appendChild(this.inputField);
       this.valueCell.appendChild(form);
-      document.l10n.setAttributes(this.editButton, "about-config-pref-save");
-      this.editButton.className = "primary button-save";
       this.editButton.setAttribute("form", "form-edit");
     }
     this.editButton.disabled = this.isLocked;
     if (!this.isLocked && this.hasUserValue) {
       if (!this.resetButton) {
         this.resetButton = document.createElement("button");
         this.resetCell.appendChild(this.resetButton);
       }
@@ -149,17 +195,17 @@ class PrefRow {
     }
     gPrefInEdit = this;
     this.editing = true;
     this.refreshElement();
     this.inputField.focus();
   }
 
   save() {
-    if (this.value.constructor.name == "Number") {
+    if (this.type == "Number") {
       if (!this.inputField.reportValidity()) {
         return;
       }
       Services.prefs.setIntPref(this.name, parseInt(this.inputField.value));
     } else {
       Services.prefs.setStringPref(this.name, this.inputField.value);
     }
     this.refreshValue();
@@ -214,100 +260,64 @@ function loadPrefs() {
     let prefRow = event.target.closest("tr");
     let pref = gElementToPrefMap.get(prefRow);
     let button = event.target.closest("button");
     if (button.classList.contains("button-reset")) {
       Services.prefs.clearUserPref(pref.name);
       pref.refreshValue();
       pref.refreshElement();
       pref.editButton.focus();
-    } else if (button.classList.contains("add-true")) {
-      addNewPref(prefRow.firstChild.innerHTML, true);
-    } else if (button.classList.contains("add-false")) {
-      addNewPref(prefRow.firstChild.innerHTML, false);
-    } else if (button.classList.contains("add-Number") ||
-      button.classList.contains("add-String")) {
-      addNewPref(prefRow.firstChild.innerHTML,
-        button.classList.contains("add-Number") ? 0 : "").edit();
+    } else if (button.classList.contains("button-add")) {
+      Preferences.set(pref.name, pref.value);
+      pref.refreshValue();
+      pref.refreshElement();
+      if (pref.type != "Boolean") {
+        pref.edit();
+      }
     } else if (button.classList.contains("button-toggle")) {
       Services.prefs.setBoolPref(pref.name, !pref.value);
       pref.refreshValue();
       pref.refreshElement();
     } else if (button.classList.contains("button-edit")) {
       pref.edit();
     } else if (button.classList.contains("button-save")) {
       pref.save();
     } else {
+      pref.editing = false;
       Services.prefs.clearUserPref(pref.name);
       gExistingPrefs.delete(pref.name);
-      prefRow.remove();
+      pref.refreshValue();
+      pref.refreshElement();
     }
   });
 
   filterPrefs();
 }
 
 function filterPrefs() {
   if (gPrefInEdit) {
     gPrefInEdit.endEdit();
   }
 
   let substring = document.getElementById("search").value.trim();
-  document.getElementById("prefs").textContent = "";
   let prefArray = [...gExistingPrefs.values()];
   if (substring) {
     prefArray = prefArray.filter(pref => pref.name.includes(substring));
   }
   prefArray.sort((a, b) => a.name > b.name);
-  if (substring && !gExistingPrefs.has(substring)) {
-    document.getElementById("prefs").appendChild(createNewPrefFragment(substring));
-  }
-  let fragment = createPrefsFragment(prefArray);
-  document.getElementById("prefs").appendChild(fragment);
-}
-
-function createPrefsFragment(prefArray) {
+  let prefsElement = document.getElementById("prefs");
   let fragment = document.createDocumentFragment();
   for (let pref of prefArray) {
     fragment.appendChild(pref.element);
   }
-  return fragment;
-}
-
-function createNewPrefFragment(name) {
-  let fragment = document.createDocumentFragment();
-  let row = document.createElement("tr");
-  row.classList.add("has-user-value");
-  row.setAttribute("aria-label", name);
-  let nameCell = document.createElement("td");
-  nameCell.className = "cell-name";
-  nameCell.append(name);
-  row.appendChild(nameCell);
-
-  let valueCell = document.createElement("td");
-  valueCell.classList.add("cell-value");
-  let guideText = document.createElement("span");
-  document.l10n.setAttributes(guideText, "about-config-pref-add");
-  valueCell.appendChild(guideText);
-  for (let item of ["true", "false", "Number", "String"]) {
-    let optionBtn = document.createElement("button");
-    optionBtn.textContent = item;
-    optionBtn.classList.add("add-" + item);
-    valueCell.appendChild(optionBtn);
+  if (substring && !gExistingPrefs.has(substring)) {
+    fragment.appendChild((new PrefRow(substring)).element);
   }
-  row.appendChild(valueCell);
-
-  let editCell = document.createElement("td");
-  row.appendChild(editCell);
-
-  let buttonCell = document.createElement("td");
-  row.appendChild(buttonCell);
-
-  fragment.appendChild(row);
-  return fragment;
+  prefsElement.textContent = "";
+  prefsElement.appendChild(fragment);
 }
 
 function prefHasDefaultValue(name) {
   try {
     switch (Services.prefs.getPrefType(name)) {
       case Ci.nsIPrefBranch.PREF_STRING:
         gDefaultBranch.getStringPref(name);
         return true;
@@ -316,16 +326,8 @@ function prefHasDefaultValue(name) {
         return true;
       case Ci.nsIPrefBranch.PREF_BOOL:
         gDefaultBranch.getBoolPref(name);
         return true;
     }
   } catch (ex) {}
   return false;
 }
-
-function addNewPref(name, value) {
-  Preferences.set(name, value);
-  let pref = new PrefRow(name);
-  gExistingPrefs.set(name, pref);
-  filterPrefs();
-  return pref;
-}
--- a/browser/components/aboutconfig/content/aboutconfig.notftl
+++ b/browser/components/aboutconfig/content/aboutconfig.notftl
@@ -8,14 +8,14 @@ about-config-warning-text = Changing the
 about-config-warning-checkbox = Annoy me again, please!
 about-config-warning-button = I accept the risk
 
 about-config-title = about:config
 
 about-config-search =
     .placeholder = Search
 
-about-config-pref-add = Add as:
+about-config-pref-add = Add
 about-config-pref-toggle = Toggle
 about-config-pref-edit = Edit
 about-config-pref-save = Save
 about-config-pref-reset = Reset
 about-config-pref-delete = Delete
--- a/browser/components/aboutconfig/test/browser/browser_edit.js
+++ b/browser/components/aboutconfig/test/browser/browser_edit.js
@@ -13,46 +13,84 @@ add_task(async function setup() {
   registerCleanupFunction(() => {
     Services.prefs.clearUserPref(PREF_BOOLEAN_DEFAULT_TRUE);
     Services.prefs.clearUserPref(PREF_NUMBER_DEFAULT_ZERO);
     Services.prefs.clearUserPref(PREF_STRING_DEFAULT_EMPTY);
   });
 });
 
 add_task(async function test_add_user_pref() {
+  const PREF_NEW = "test.aboutconfig.new";
+  Assert.equal(Services.prefs.getPrefType(PREF_NEW),
+               Ci.nsIPrefBranch.PREF_INVALID);
+
   await AboutConfigTest.withNewTab(async function() {
-    Assert.ok(!Services.prefs.getChildList("").find(pref => pref == "testPref"));
+    // The row for a new preference appears when searching for its name.
+    Assert.ok(!this.getRow(PREF_NEW));
+    this.search(PREF_NEW);
+    let row = this.getRow(PREF_NEW);
+    Assert.ok(row.hasClass("deleted"));
 
-    for (let [buttonSelector, expectedValue] of [
-      [".add-true", true],
-      [".add-false", false],
-      [".add-Number", 0],
-      [".add-String", ""],
+    for (let [radioIndex, expectedValue, expectedEditingMode] of [
+      [0, true, false],
+      [1, 0, true],
+      [2, "", true],
     ]) {
-      this.search("testPref");
-      this.document.querySelector("#prefs button" + buttonSelector).click();
-      Assert.ok(Services.prefs.getChildList("").find(pref => pref == "testPref"));
-      Assert.ok(Preferences.get("testPref") === expectedValue);
-      this.document.querySelector("#prefs button[data-l10n-id='about-config-pref-delete']").click();
+      // Adding the preference should set the default for the data type.
+      row.element.querySelectorAll("input")[radioIndex].click();
+      row.editColumnButton.click();
+      Assert.ok(!row.hasClass("deleted"));
+      Assert.ok(Preferences.get(PREF_NEW) === expectedValue);
+
+      // Number and String preferences should be in edit mode.
+      Assert.equal(!!row.valueInput, expectedEditingMode);
+
+      // Repeat the search to verify that the preference remains.
+      this.search(PREF_NEW);
+      row = this.getRow(PREF_NEW);
+      Assert.ok(!row.hasClass("deleted"));
+      Assert.ok(!row.valueInput);
+
+      // Reset the preference, then continue by adding a different type.
+      row.resetColumnButton.click();
+      Assert.equal(Services.prefs.getPrefType(PREF_NEW),
+                   Ci.nsIPrefBranch.PREF_INVALID);
     }
   });
 });
 
 add_task(async function test_delete_user_pref() {
-  Services.prefs.setBoolPref("userAddedPref", true);
-  await AboutConfigTest.withNewTab(async function() {
-    let row = this.getRow("userAddedPref");
-    row.resetColumnButton.click();
-    Assert.ok(!this.getRow("userAddedPref"));
-    Assert.ok(!Services.prefs.getChildList("").includes("userAddedPref"));
+  const PREF_NEW = "test.aboutconfig.new";
 
-    // Search for nothing to test gPrefArray
-    this.search();
-    Assert.ok(!this.getRow("userAddedPref"));
-  });
+  for (let [radioIndex, testValue] of [
+    [0, false],
+    [1, -1],
+    [2, "value"],
+  ]) {
+    Preferences.set(PREF_NEW, testValue);
+    await AboutConfigTest.withNewTab(async function() {
+      // Deleting the preference should keep the row.
+      let row = this.getRow(PREF_NEW);
+      row.resetColumnButton.click();
+      Assert.ok(row.hasClass("deleted"));
+      Assert.equal(Services.prefs.getPrefType(PREF_NEW),
+                   Ci.nsIPrefBranch.PREF_INVALID);
+
+      // Re-adding the preference should keep the same value.
+      Assert.ok(row.element.querySelectorAll("input")[radioIndex].checked);
+      row.editColumnButton.click();
+      Assert.ok(!row.hasClass("deleted"));
+      Assert.ok(Preferences.get(PREF_NEW) === testValue);
+
+      // Searching again after deleting should remove the row.
+      row.resetColumnButton.click();
+      this.search();
+      Assert.ok(!this.getRow(PREF_NEW));
+    });
+  }
 });
 
 add_task(async function test_reset_user_pref() {
   await SpecialPowers.pushPrefEnv({
     "set": [
       [PREF_BOOLEAN_DEFAULT_TRUE, false],
       [PREF_STRING_LOCALIZED_MISSING, "user-value"],
     ],
@@ -143,16 +181,13 @@ add_task(async function test_modify() {
       row.editColumnButton.click();
       Assert.equal(Preferences.get(prefName), "42");
       Assert.equal(row.value, "42");
       Assert.ok(row.hasClass("has-user-value"));
       // Reset or delete the preference while editing.
       row.editColumnButton.click();
       Assert.equal(row.valueInput.value, Preferences.get(prefName));
       row.resetColumnButton.click();
-      if (willDelete) {
-        Assert.ok(!this.getRow(prefName));
-      } else {
-        Assert.ok(!row.hasClass("has-user-value"));
-      }
+      Assert.ok(!row.hasClass("has-user-value"));
+      Assert.equal(row.hasClass("deleted"), willDelete);
     }
   });
 });
--- a/browser/components/aboutconfig/test/browser/head.js
+++ b/browser/components/aboutconfig/test/browser/head.js
@@ -37,18 +37,18 @@ class AboutConfigRowTest {
   /**
    * Text input field when the row is in edit mode.
    */
   get valueInput() {
     return this.querySelector("td.cell-value input");
   }
 
   /**
-   * This is normally "edit" or "toggle" based on the preference type, or "save"
-   * when the row is in edit mode.
+   * This is normally "edit" or "toggle" based on the preference type, "save"
+   * when the row is in edit mode, or "add" when the preference does not exist.
    */
   get editColumnButton() {
     return this.querySelector("td.cell-edit > button");
   }
 
   /**
    * This can be "reset" or "delete" based on whether a default exists.
    */