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

pbro did r+ this patch some time back but it kept breaking try. The only differences between then and now are:

1. `devtools/client/storage/ui.js:859` was reverted to using "" instead of undefined to fix test issues.
2. A bad check for mathematical values was replaced with a `MATH_REGEX` check `devtools/client/storage/ui.js:64,941-943`

A new try run is here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0212fe328f13991ece396bccb44668d821cb4218&group_state=expanded

Differential Revision: https://phabricator.services.mozilla.com/D18578

--HG--
extra : moz-landing-system : lando

devtools/client/storage/test/browser.ini

@@ -21,6 +21,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
@@ -70,6 +71,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(async function() {
+  await openTabAndSetupStorage(MAIN_DOMAIN + "storage-sidebar-parsetree.html");
+
+  await selectTreeItem(["localStorage", "http://test1.example.org"]);
+
+  for (const test of testCases) {
+    const { parseTreeVisible, row } = test;
+
+    await selectTableItem(row);
+
+    sidebarParseTreeVisible(parseTreeVisible);
+  }
+
+  await finishTests();
+});

devtools/client/storage/test/head.js

@@ -535,7 +535,7 @@ async function selectTableItem(id) {
                  " .table-widget-cell[value='" + id + "']";
   const target = gPanelWindow.document.querySelector(selector);
 
-  ok(target, "table item found with ids " + id);
+  ok(target, `row found with id "${id}"`);
 
   if (!target) {
     showAvailableIds();
@@ -543,6 +543,7 @@ async function selectTableItem(id) {
 
   const updated = gUI.once("sidebar-updated");
 
+  info(`selecting row "${id}"`);
   await click(target);
   await updated;
 }
@@ -968,6 +969,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

@@ -23,7 +23,9 @@ loader.lazyRequireGetter(this, "TreeWidget",
 loader.lazyRequireGetter(this, "TableWidget",
                          "devtools/client/shared/widgets/TableWidget", true);
 loader.lazyImporter(this, "VariablesView",
-  "resource://devtools/client/shared/widgets/VariablesView.jsm");
+                    "resource://devtools/client/shared/widgets/VariablesView.jsm");
+loader.lazyRequireGetter(this, "validator",
+                         "devtools/client/shared/vendor/stringvalidator/validator");
 
 /**
  * Localization convenience methods.
@@ -59,6 +61,7 @@ const COOKIE_KEY_MAP = {
 };
 
 const SAFE_HOSTS_PREFIXES_REGEX = /^(about:|https?:|file:|moz-extension:)/;
+const MATH_REGEX = /(?:(?:^|[-+_*/])(?:\s*-?\d+(\.\d+)?(?:[eE][+-]?\d+)?\s*))+$/;
 
 // Maximum length of item name to show in context menu label - will be
 // trimmed with ellipsis if it's longer.
@@ -827,31 +830,29 @@ class StorageUI {
     const 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;
     }
 
     const jsonObject = Object.create(null);
     const view = this.view;
-    jsonObject[name] = json;
+    jsonObject[name] = obj;
     const valueScope = view.getScopeAtIndex(1) ||
                       view.addScope(L10N.getStr("storage.parsedValue.label"));
     valueScope.expanded = true;
@@ -904,13 +905,54 @@ class StorageUI {
       const word = `[^${p}]*`;
       const wordList = `(${word}${p})+${word}`;
       const regex = new RegExp(`^${wordList}$`);
-      if (value.match && value.match(regex)) {
-        return value.split(p.replace(/\\*/g, ""));
+
+      if (regex.test(value)) {
+        const 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(value) {
+    const 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 (MATH_REGEX.test(value)) {
+      return false;
+    }
+
+    // Check for any other types that shouldn't be parsed.
+    for (const 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.