Bug 1356943 - Only show a tree in the storage sidebar when it is useful r=pbro

MozReview-Commit-ID: HUg9ouWESx

--HG--
extra : rebase_source : 21c33b71c4f45755f32f086611e0a299d77144f5

devtools/client/storage/test/browser.ini

@@ -17,6 +17,7 @@ support-files =
   storage-secured-iframe.html
   storage-secured-iframe-usercontextid.html
   storage-sessionstorage.html
+  storage-sidebar-parsetree.html
   storage-unsecured-iframe.html
   storage-unsecured-iframe-usercontextid.html
   storage-updates.html
@@ -59,6 +60,7 @@ tags = usercontextid
 [browser_storage_sessionstorage_add.js]
 [browser_storage_sessionstorage_edit.js]
 [browser_storage_sidebar.js]
+[browser_storage_sidebar_parsetree.js]
 [browser_storage_sidebar_toggle.js]
 [browser_storage_sidebar_update.js]
 [browser_storage_values.js]

devtools/client/storage/test/browser_storage_sidebar_parsetree.js

@@ -0,0 +1,115 @@
+/* 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/. */
+
+// Test to verify that the sidebar parsetree is used for only values it makes sense to
+// parse into a tree.
+
+"use strict";
+
+const testCases = [
+  {
+    row: "ampersand",
+    parseTreeVisible: true
+  },
+  {
+    row: "asterisk",
+    parseTreeVisible: true
+  },
+  {
+    row: "base64",
+    parseTreeVisible: false
+  },
+  {
+    row: "boolean",
+    parseTreeVisible: false
+  },
+  {
+    row: "colon",
+    parseTreeVisible: true
+  },
+  {
+    row: "color",
+    parseTreeVisible: false
+  },
+  {
+    row: "comma",
+    parseTreeVisible: true
+  },
+  {
+    row: "dataURI",
+    parseTreeVisible: false
+  },
+  {
+    row: "date",
+    parseTreeVisible: false
+  },
+  {
+    row: "email",
+    parseTreeVisible: false
+  },
+  {
+    row: "equals",
+    parseTreeVisible: true
+  },
+  {
+    row: "FQDN",
+    parseTreeVisible: false
+  },
+  {
+    row: "hash",
+    parseTreeVisible: true
+  },
+  {
+    row: "IP",
+    parseTreeVisible: false
+  },
+  {
+    row: "MacAddress",
+    parseTreeVisible: false
+  },
+  {
+    row: "maths",
+    parseTreeVisible: false
+  },
+  {
+    row: "numbers",
+    parseTreeVisible: false
+  },
+  {
+    row: "period",
+    parseTreeVisible: true
+  },
+  {
+    row: "SemVer",
+    parseTreeVisible: false
+  },
+  {
+    row: "tilde",
+    parseTreeVisible: true
+  },
+  {
+    row: "URL",
+    parseTreeVisible: false
+  },
+  {
+    row: "URL2",
+    parseTreeVisible: false
+  },
+];
+
+add_task(function* () {
+  yield openTabAndSetupStorage(MAIN_DOMAIN + "storage-sidebar-parsetree.html");
+
+  yield selectTreeItem(["localStorage", "http://test1.example.org"]);
+
+  for (let test of testCases) {
+    let { parseTreeVisible, row } = test;
+
+    yield selectTableItem(row);
+
+    sidebarParseTreeVisible(parseTreeVisible);
+  }
+
+  yield finishTests();
+});

devtools/client/storage/test/head.js

@@ -536,7 +536,7 @@ function* selectTableItem(id) {
                  " .table-widget-cell[value='" + id + "']";
   let target = gPanelWindow.document.querySelector(selector);
 
-  ok(target, "table item found with ids " + id);
+  ok(target, `row found with id "${id}"`);
 
   if (!target) {
     showAvailableIds();
@@ -544,6 +544,7 @@ function* selectTableItem(id) {
 
   let updated = gUI.once("sidebar-updated");
 
+  info(`selecting row "${id}"`);
   yield click(target);
   yield updated;
 }
@@ -962,6 +963,20 @@ function sidebarToggleVisible() {
   return !gUI.sidebarToggleBtn.hidden;
 }
 
+/**
+ * Check whether the variables view in the sidebar contains a tree.
+ *
+ * @param  {Boolean} state
+ *         Should a tree be visible?
+ */
+function sidebarParseTreeVisible(state) {
+  if (state) {
+    ok(gUI.view._currHierarchy.size > 2, "Parse tree should be visible.");
+  } else {
+    ok(gUI.view._currHierarchy.size <= 2, "Parse tree should not be visible.");
+  }
+}
+
 /**
  * Add an item.
  * @param  {Array} store

devtools/client/storage/test/storage-sidebar-parsetree.html

@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+    <title>Storage inspector sidebar parsetree test</title>
+    <script type="application/javascript">
+      "use strict";
+
+      function setup() {
+        // These values should not be parsed into a tree.
+        localStorage.setItem("base64", "aGVsbG93b3JsZA==");
+        localStorage.setItem("boolean", "true");
+        localStorage.setItem("color", "#ff0034");
+        localStorage.setItem("dataURI", "data:,Hello World!");
+        localStorage.setItem("date", "2009-05-19 14:39:22-01");
+        localStorage.setItem("email", "foo@bar.co.uk");
+        localStorage.setItem("FQDN", "xn--froschgrn-x9a.co.uk");
+        localStorage.setItem("IP", "192.168.1.1");
+        localStorage.setItem("MacAddress", "01:AB:03:04:05:06");
+        localStorage.setItem("maths", "9-1");
+        localStorage.setItem("numbers", "10,123,456");
+        localStorage.setItem("SemVer", "1.0.4");
+        localStorage.setItem("URL", "www.google.co.uk");
+        localStorage.setItem("URL2", "http://www.google.co.uk");
+
+        // These values should be parsed into a tree.
+        localStorage.setItem("ampersand", "a&b&c&d&e&f&g");
+        localStorage.setItem("asterisk", "a*b*c*d*e*f*g");
+        localStorage.setItem("colon", "a:b:c:d:e:f:g");
+        localStorage.setItem("comma", "a,b,c,d,e,f,g");
+        localStorage.setItem("equals", "a=b=c=d=e=f=g");
+        localStorage.setItem("hash", "a#b#c#d#e#f#g");
+        localStorage.setItem("period", "a.b.c.d.e.f.g");
+        localStorage.setItem("tilde", "a~b~c~d~e~f~g");
+      }
+    </script>
+  </head>
+  <body onload="setup()">
+  </body>
+</html>

devtools/client/storage/ui.js

@@ -24,6 +24,9 @@ loader.lazyRequireGetter(this, "TableWidget",
                          "devtools/client/shared/widgets/TableWidget", true);
 loader.lazyRequireGetter(this, "ViewHelpers",
                          "devtools/client/shared/widgets/view-helpers");
+loader.lazyRequireGetter(this, "validator",
+                         "devtools/client/shared/vendor/stringvalidator/validator");
+
 loader.lazyImporter(this, "VariablesView",
   "resource://devtools/client/shared/widgets/VariablesView.jsm");
 
@@ -763,35 +766,33 @@ StorageUI.prototype = {
     let value = (decodedValue && decodedValue !== originalValue)
       ? decodedValue : originalValue;
 
-    let json = null;
-    try {
-      json = JSOL.parse(value);
-    } catch (ex) {
-      json = null;
-    }
-
-    if (!json && value) {
-      json = this._extractKeyValPairs(value);
-    }
-
-    // return if json is null, or same as value, or just a string.
-    if (!json || json == value || typeof json == "string") {
+    if (!this._shouldParse(value)) {
       return;
     }
 
-    // One special case is a url which gets separated as key value pair on :
-    if ((json.length == 2 || Object.keys(json).length == 1) &&
-        ((json[0] || Object.keys(json)[0]) + "").match(/^(http|file|ftp)/)) {
+    let obj = null;
+    try {
+      obj = JSOL.parse(value);
+    } catch (ex) {
+      obj = null;
+    }
+
+    if (!obj && value) {
+      obj = this._extractKeyValPairs(value);
+    }
+
+    // return if obj is null, or same as value, or just a string.
+    if (!obj || obj === value || typeof obj === "string") {
       return;
     }
 
     let jsonObject = Object.create(null);
     let view = this.view;
-    jsonObject[name] = json;
+    jsonObject[name] = obj;
     let valueScope = view.getScopeAtIndex(1) ||
                      view.addScope(L10N.getStr("storage.parsedValue.label"));
     valueScope.expanded = true;
-    let jsonVar = valueScope.addItem("", Object.create(null), {relaxed: true});
+    let jsonVar = valueScope.addItem(undefined, Object.create(null), {relaxed: true});
     jsonVar.expanded = true;
     jsonVar.twisty = true;
     jsonVar.populate(jsonObject, {expanded: true});
@@ -835,18 +836,60 @@ StorageUI.prototype = {
         }
       }
     }
+
     // Testing for array
     for (let p of separators) {
       let word = `[^${p}]*`;
       let wordList = `(${word}${p})+${word}`;
       let regex = new RegExp(`^${wordList}$`);
-      if (value.match && value.match(regex)) {
-        return value.split(p.replace(/\\*/g, ""));
+
+      if (regex.test(value)) {
+        let pNoBackslash = p.replace(/\\*/g, "");
+        return value.split(pNoBackslash);
       }
     }
     return null;
   },
 
+  /**
+   * Check whether the value string represents something that should be displayed as text.
+   * If so then it shouldn't be parsed into a tree.
+   *
+   * @param  {String} value
+   *         The value to be parsed.
+   */
+  _shouldParse: function (value) {
+    let validators = [
+      "isBase64",
+      "isBoolean",
+      "isCurrency",
+      "isDataURI",
+      "isEmail",
+      "isFQDN",
+      "isHexColor",
+      "isIP",
+      "isISO8601",
+      "isMACAddress",
+      "isSemVer",
+      "isURL"
+    ];
+
+    // Check for minus calculations e.g. 8-3 because otherwise 5 will be displayed.
+    if (validator.whitelist(value, "0-9-")) {
+      return false;
+    }
+
+    // Check for any other types that shouldn't be parsed.
+    for (let test of validators) {
+      if (validator[test](value)) {
+        return false;
+      }
+    }
+
+    // Seems like this is data that should be parsed.
+    return true;
+  },
+
   /**
    * Select handler for the storage tree. Fetches details of the selected item
    * from the storage details and populates the storage tree.