Bug 1251084 - Remove WebConsoleUtils.abbreviateSourceURL with source-utils function. r=linclark

devtools/client/locales/en-US/webconsole.properties

@@ -98,11 +98,6 @@ stacktrace.anonymousFunction=<anonymous>
 # %S is the "Async Cause" of the frame.
 stacktrace.asyncStack=(Async: %S)
 
-# LOCALIZATION NOTE (unknownLocation): this string is used to
-# display messages with sources that have an unknown location, eg. from
-# console.trace() calls.
-unknownLocation=<unknown>
-
 # LOCALIZATION NOTE (timerStarted): this string is used to display the result
 # of the console.time() call. Parameters: %S is the name of the timer.
 timerStarted=%S: timer started

devtools/client/memory/components/shortest-paths.js

@@ -12,11 +12,6 @@ const { isSavedFrame } = require("devtools/shared/DevToolsUtils");
 const { getSourceNames } = require("devtools/client/shared/source-utils");
 const { L10N } = require("../utils");
 
-const { ViewHelpers } = require("resource://devtools/client/shared/widgets/ViewHelpers.jsm");
-const COMPONENTS_STRINGS_URI = "chrome://devtools/locale/components.properties";
-const componentsL10N = new ViewHelpers.L10N(COMPONENTS_STRINGS_URI);
-const UNKNOWN_SOURCE_STRING = componentsL10N.getStr("frame.unknownSource");
-
 const GRAPH_DEFAULTS = {
   translate: [20, 20],
   scale: 1
@@ -33,7 +28,7 @@ function stringifyLabel(label, id) {
     const piece = label[i];
 
     if (isSavedFrame(piece)) {
-      const { short } = getSourceNames(piece.source, UNKNOWN_SOURCE_STRING);
+      const { short } = getSourceNames(piece.source);
       sanitized[i] = `${piece.functionDisplayName} @ ${short}:${piece.line}:${piece.column}`;
     } else if (piece === NO_STACK) {
       sanitized[i] = L10N.getStr("tree-item.nostack");

devtools/client/performance/modules/logic/marker-utils.js

@@ -13,7 +13,7 @@ const { Cu, Ci } = require("chrome");
 const Services = require("Services");
 const { L10N } = require("devtools/client/performance/modules/global");
 const { TIMELINE_BLUEPRINT } = require("devtools/client/performance/modules/markers");
-const WebConsoleUtils = require("devtools/shared/webconsole/utils");
+const { getSourceNames } = require("devtools/client/shared/source-utils");
 const SHOW_TRIGGER_FOR_GC_TYPES_PREF = "devtools.performance.ui.show-triggers-for-gc-types";
 
 /**
@@ -259,7 +259,7 @@ const DOM = {
 
         let urlNode = doc.createElement("label");
         urlNode.className = "filename";
-        urlNode.setAttribute("value", WebConsoleUtils.Utils.abbreviateSourceURL(url));
+        urlNode.setAttribute("value", getSourceNames(url).short);
         let lineNode = doc.createElement("label");
         lineNode.className = "line-number";
         lineNode.setAttribute("value", `:${line}`);

devtools/client/shared/components/frame.js

@@ -2,13 +2,10 @@
  * 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/. */
 
-const { Cu } = require("chrome");
-Cu.import("resource://devtools/client/shared/widgets/ViewHelpers.jsm");
-const STRINGS_URI = "chrome://devtools/locale/components.properties";
-const L10N = new ViewHelpers.L10N(STRINGS_URI);
 const { DOM: dom, createClass, PropTypes } = require("devtools/client/shared/vendor/react");
 const { getSourceNames } = require("devtools/client/shared/source-utils");
-const UNKNOWN_SOURCE_STRING = L10N.getStr("frame.unknownSource");
+const { L10N } = require("resource://devtools/client/shared/widgets/ViewHelpers.jsm").ViewHelpers;
+const l10n = new L10N("chrome://devtools/locale/components.properties");
 
 const Frame = module.exports = createClass({
   displayName: "Frame",
@@ -39,7 +36,7 @@ const Frame = module.exports = createClass({
   render() {
     let { onClick, frame, showFunctionName, showHost } = this.props;
 
-    const { short, long, host } = getSourceNames(frame.source, UNKNOWN_SOURCE_STRING);
+    const { short, long, host } = getSourceNames(frame.source);
 
     let tooltip = `${long}:${frame.line}`;
     if (frame.column) {
@@ -51,7 +48,7 @@ const Frame = module.exports = createClass({
       sourceString += `:${frame.column}`;
     }
 
-    let onClickTooltipString = L10N.getFormatStr("frame.viewsourceindebugger", sourceString);
+    let onClickTooltipString = l10n.getFormatStr("frame.viewsourceindebugger", sourceString);
 
     let fields = [
       dom.a({

devtools/client/shared/source-utils.js

@@ -4,10 +4,16 @@
 "use strict";
 
 const { URL } = require("sdk/url");
+const { Cu } = require("chrome");
+Cu.import("resource://devtools/client/shared/widgets/ViewHelpers.jsm");
+const STRINGS_URI = "chrome://devtools/locale/components.properties";
+const L10N = new ViewHelpers.L10N(STRINGS_URI);
+const UNKNOWN_SOURCE_STRING = L10N.getStr("frame.unknownSource");
 
 // Character codes used in various parsing helper functions.
 const CHAR_CODE_A = "a".charCodeAt(0);
 const CHAR_CODE_C = "c".charCodeAt(0);
+const CHAR_CODE_D = "d".charCodeAt(0);
 const CHAR_CODE_E = "e".charCodeAt(0);
 const CHAR_CODE_F = "f".charCodeAt(0);
 const CHAR_CODE_H = "h".charCodeAt(0);
@@ -26,6 +32,8 @@ const CHAR_CODE_SLASH = "/".charCodeAt(0);
 
 // The cache used in the `nsIURL` function.
 const gURLStore = new Map();
+// The cache used in the `getSourceNames` function.
+const gSourceNamesStore = new Map();
 
 /**
  * Takes a string and returns an object containing all the properties
@@ -81,17 +89,38 @@ function parseURL(location) {
  *
  * @param {String} source
  *        The source to parse. Can be a URI or names like "(eval)" or "self-hosted".
- * @param {String} unknownSourceString
- *        The string to use if no valid source name can be generated.
  * @return {Object}
  *         An object with the following properties:
  *           - {String} short: A short name for the source.
- *           - {String} long: The full, long name for the source.
+ *             - "http://page.com/test.js#go?q=query" -> "test.js"
+ *           - {String} long: The full, long name for the source, with hash/query stripped.
+ *             - "http://page.com/test.js#go?q=query" -> "http://page.com/test.js"
  *           - {String?} host: If available, the host name for the source.
+ *             - "http://page.com/test.js#go?q=query" -> "page.com"
  */
-function getSourceNames (source, unknownSourceString) {
+function getSourceNames (source) {
+  let data = gSourceNamesStore.get(source);
+
+  if (data) {
+    return data;
+  }
+
   let short, long, host;
   const sourceStr = source ? String(source) : "";
+
+  // If `data:...` uri
+  if (isDataScheme(sourceStr)) {
+    let commaIndex = sourceStr.indexOf(",");
+    if (commaIndex > -1) {
+      // The `short` name for a data URI becomes `data:` followed by the actual
+      // encoded content, omitting the MIME type, and charset.
+      let short = `data:${sourceStr.substring(commaIndex + 1)}`.slice(0, 100);
+      let result = { short, long: sourceStr };
+      gSourceNamesStore.set(source, result);
+      return result;
+    }
+  }
+
   const parsedUrl = parseURL(sourceStr);
 
   if (!parsedUrl) {
@@ -99,19 +128,35 @@ function getSourceNames (source, unknownSourceString) {
     long = sourceStr;
     short = sourceStr.slice(0, 100);
   } else {
-    short = parsedUrl.fileName;
-    long = parsedUrl.href;
     host = parsedUrl.host;
+
+    long = parsedUrl.href;
+    if (parsedUrl.hash) {
+      long = long.replace(parsedUrl.hash, "");
+    }
+    if (parsedUrl.search) {
+      long = long.replace(parsedUrl.search, "");
+    }
+
+    short = parsedUrl.fileName;
+    // If `short` is just a slash, and we actually have a path,
+    // strip the slash and parse again to get a more useful short name.
+    // e.g. "http://foo.com/bar/" -> "bar", rather than "/"
+    if (short === "/" && parsedUrl.pathname !== "/") {
+      short = parseURL(long.replace(/\/$/, "")).fileName;
+    }
   }
 
   if (!short) {
     if (!long) {
-      long = unknownSourceString;
+      long = UNKNOWN_SOURCE_STRING;
     }
     short = long.slice(0, 100);
   }
 
-  return { short, long, host };
+  let result = { short, long, host };
+  gSourceNamesStore.set(source, result);
+  return result;
 }
 
 // For the functions below, we assume that we will never access the location
@@ -126,6 +171,14 @@ function isColonSlashSlash(location, i=0) {
          location.charCodeAt(++i) === CHAR_CODE_SLASH;
 }
 
+function isDataScheme(location, i=0) {
+  return location.charCodeAt(i)   === CHAR_CODE_D &&
+         location.charCodeAt(++i) === CHAR_CODE_A &&
+         location.charCodeAt(++i) === CHAR_CODE_T &&
+         location.charCodeAt(++i) === CHAR_CODE_A &&
+         location.charCodeAt(++i) === CHAR_CODE_COLON;
+}
+
 function isContentScheme(location, i=0) {
   let firstChar = location.charCodeAt(i);
 
@@ -208,3 +261,4 @@ exports.parseURL = parseURL;
 exports.getSourceNames = getSourceNames;
 exports.isChromeScheme = isChromeScheme;
 exports.isContentScheme = isContentScheme;
+exports.isDataScheme = isDataScheme;

devtools/client/shared/test/unit/test_source-utils.js

@@ -57,21 +57,89 @@ add_task(function* () {
   }
 });
 
+// Test `sourceUtils.isDataScheme`.
+add_task(function* () {
+  let dataURI = "data:text/html;charset=utf-8,<!DOCTYPE html></html>";
+  ok(sourceUtils.isDataScheme(dataURI), `${dataURI} correctly identified as data scheme`);
+
+  for (let url of CHROME_URLS) {
+    ok(!sourceUtils.isDataScheme(url), `${url} correctly identified as not data scheme`);
+  }
+  for (let url of CONTENT_URLS) {
+    ok(!sourceUtils.isDataScheme(url), `${url} correctly identified as not data scheme`);
+  }
+});
+
 // Test `sourceUtils.getSourceNames`.
 add_task(function* () {
-  const url = "http://example.com:8888/foo/bar/baz.js";
-  let results = sourceUtils.getSourceNames(url);
-  equal(results.short, "baz.js");
-  equal(results.long, url);
-  equal(results.host, "example.com:8888");
 
-  results = sourceUtils.getSourceNames("self-hosted");
-  equal(results.short, "self-hosted");
-  equal(results.long, "self-hosted");
-  equal(results.host, undefined);
+  // Check length
+  let longMalformedURL = `example.com${new Array(100).fill("/a").join("")}/file.js`;
+  ok(sourceUtils.getSourceNames(longMalformedURL).short.length <= 100,
+    "`short` names are capped at 100 characters");
 
-  results = sourceUtils.getSourceNames("", "<unknown>");
-  equal(results.short, "<unknown>");
-  equal(results.long, "<unknown>");
-  equal(results.host, undefined);
+  testAbbreviation("self-hosted", "self-hosted", "self-hosted");
+  testAbbreviation("", "(unknown)", "(unknown)");
+
+  // Test shortening data URIs, stripping mime/charset
+  testAbbreviation("data:text/html;charset=utf-8,<!DOCTYPE html></html>",
+                   "data:<!DOCTYPE html></html>",
+                   "data:text/html;charset=utf-8,<!DOCTYPE html></html>");
+
+  let longDataURI = `data:image/png;base64,${new Array(100).fill("a").join("")}`;
+  let longDataURIShort = sourceUtils.getSourceNames(longDataURI).short;
+
+  // Test shortening data URIs and that the `short` result is capped
+  ok(longDataURIShort.length <= 100,
+    "`short` names are capped at 100 characters for data URIs");
+  equal(longDataURIShort.substr(0, 10), "data:aaaaa",
+    "truncated data URI short names still have `data:...`");
+
+  testAbbreviation("http://example.com/foo/bar/baz/boo.js",
+                   "boo.js",
+                   "http://example.com/foo/bar/baz/boo.js",
+                   "example.com");
+
+  // Check query and hash and port
+  testAbbreviation("http://example.com:8888/foo/bar/baz.js?q=query#go",
+                   "baz.js",
+                   "http://example.com:8888/foo/bar/baz.js",
+                   "example.com:8888");
+
+  // Trailing "/" with nothing beyond host
+  testAbbreviation("http://example.com/",
+                   "/",
+                   "http://example.com/",
+                   "example.com");
+
+  // Trailing "/"
+  testAbbreviation("http://example.com/foo/bar/",
+                   "bar",
+                   "http://example.com/foo/bar/",
+                   "example.com");
+
+  // Non-extension ending
+  testAbbreviation("http://example.com/bar",
+                   "bar",
+                   "http://example.com/bar",
+                   "example.com");
+
+  // Check query
+  testAbbreviation("http://example.com/foo.js?bar=1&baz=2",
+                   "foo.js",
+                   "http://example.com/foo.js",
+                   "example.com");
+
+  // Check query with trailing slash
+  testAbbreviation("http://example.com/foo/?bar=1&baz=2",
+                   "foo",
+                   "http://example.com/foo/",
+                   "example.com");
+
+  function testAbbreviation(source, short, long, host) {
+    let results = sourceUtils.getSourceNames(source);
+    equal(results.short, short, `${source} has correct "short" name`);
+    equal(results.long, long, `${source} has correct "long" name`);
+    equal(results.host, host, `${source} has correct "host" name`);
+  }
 });

devtools/client/shared/widgets/VariablesView.jsm

@@ -24,6 +24,7 @@ Cu.import("resource://gre/modules/Task.jsm");
 const { require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
 const DevToolsUtils = require("devtools/shared/DevToolsUtils");
 const Services = require("Services");
+const { getSourceNames } = require("devtools/client/shared/source-utils");
 const promise = require("promise");
 
 XPCOMUtils.defineLazyModuleGetter(this, "PluralForm",
@@ -3611,8 +3612,7 @@ VariablesView.stringifiers.byObjectKind = {
     let result = aGrip.class;
     let url = aGrip.preview.url;
     if (!VariablesView.isFalsy({ value: url })) {
-      result += " \u2192 " + WebConsoleUtils.abbreviateSourceURL(url,
-                             { onlyCropQuery: !concise });
+      result += ` \u2192 ${getSourceNames(url)[concise ? "short" : "long"]}`;
     }
     return result;
   },
@@ -3749,9 +3749,7 @@ VariablesView.stringifiers.byObjectKind = {
       case Ci.nsIDOMNode.DOCUMENT_NODE: {
         let result = aGrip.class;
         if (preview.location) {
-          let location = WebConsoleUtils.abbreviateSourceURL(preview.location,
-                                                            { onlyCropQuery: !concise });
-          result += " \u2192 " + location;
+          result += ` \u2192 ${getSourceNames(preview.location)[concise ? "short" : "long"]}`;
         }
 
         return result;

devtools/client/webconsole/console-output.js

@@ -27,6 +27,7 @@ const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 const STRINGS_URI = "chrome://devtools/locale/webconsole.properties";
 
 const WebConsoleUtils = require("devtools/shared/webconsole/utils").Utils;
+const { getSourceNames } = require("devtools/client/shared/source-utils");
 const l10n = new WebConsoleUtils.L10n(STRINGS_URI);
 
 const MAX_STRING_GRIP_LENGTH = 36;
@@ -2948,9 +2949,7 @@ Widgets.ObjectRenderers.add({
 
     if (!VariablesView.isFalsy({ value: url })) {
       this._text(" \u2192 ", container);
-      let shortUrl = WebConsoleUtils.abbreviateSourceURL(url, {
-        onlyCropQuery: !this.options.concise
-      });
+      let shortUrl = getSourceNames(url)[this.options.concise ? "short" : "long"];
       this._anchor(shortUrl, { href: url, appendTo: container });
     }
 

devtools/client/webconsole/test/browser.ini

@@ -194,7 +194,6 @@ skip-if = e10s && debug && os == 'win'
 [browser_repeated_messages_accuracy.js]
 [browser_result_format_as_string.js]
 [browser_warn_user_about_replaced_api.js]
-[browser_webconsole_abbreviate_source_url.js]
 [browser_webconsole_allow_mixedcontent_securityerrors.js]
 tags = mcb
 [browser_webconsole_assert.js]

devtools/client/webconsole/test/browser_webconsole_abbreviate_source_url.js

@@ -1,25 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-// Tests that source URLs are abbreviated properly for display on the right-
-// hand side of the Web Console.
-
-"use strict";
-
-function test() {
-  testAbbreviation("http://example.com/x.js", "x.js");
-  testAbbreviation("http://example.com/foo/bar/baz/boo.js", "boo.js");
-  testAbbreviation("http://example.com/foo/bar/", "bar");
-  testAbbreviation("http://example.com/foo.js?bar=1&baz=2", "foo.js");
-  testAbbreviation("http://example.com/foo/?bar=1&baz=2", "foo");
-
-  finishTest();
-}
-
-function testAbbreviation(aFullURL, aAbbreviatedURL) {
-  is(WebConsoleUtils.abbreviateSourceURL(aFullURL), aAbbreviatedURL, aFullURL +
-     " is abbreviated to " + aAbbreviatedURL);
-}
-

devtools/client/webconsole/test/test-console-output-dom-elements.html

@@ -2,7 +2,7 @@
 <html dir="ltr" lang="en-US">
 <head>
   <meta charset="utf-8">
-  <title>Test the web console output - 05</title>
+  <title>Test the web console output - dom elements</title>
   <!--
   - Any copyright is dedicated to the Public Domain.
   - http://creativecommons.org/publicdomain/zero/1.0/
@@ -11,7 +11,15 @@
 <body class="body-class" id="body-id">
   <p some-attribute="some-value">hello world!</p>
   <p id="lots-of-attributes" a b c d e f g h i j k l m n></p>
-  <iframe src="data:text/html,<p>hello from iframe</p>"></iframe>
+  <!--
+    Be sure we have a charset in our iframe's data URI, otherwise we get the following extra
+    console output message:
+    "The character encoding of a framed document was not declared. The document may appear different if viewed without the document framing it."
+    This wouldn't be a big deal, but when we look for a "<p>" in our `waitForMessage` helper,
+    this extra encoding warning line contains the data URI source, returning a message
+    that was unexpected
+  -->
+  <iframe src="data:text/html;charset=US-ASCII,<p>hello from iframe</p>"></iframe>
   <div class="some       classname      here      with       more classnames here"></div>
   <script type="text/javascript">
 function testBodyNode() {

devtools/client/webconsole/webconsole.js

@@ -10,6 +10,7 @@ const {Cc, Ci, Cu} = require("chrome");
 
 const {Utils: WebConsoleUtils, CONSOLE_WORKER_IDS} =
   require("devtools/shared/webconsole/utils");
+const { getSourceNames } = require("devtools/client/shared/source-utils");
 const promise = require("promise");
 const Debugger = require("Debugger");
 const Services = require("Services");
@@ -1746,7 +1747,7 @@ WebConsoleFrame.prototype = {
       let a = this.document.createElementNS(XHTML_NS, "a");
       a.href = "#";
       a.draggable = "false";
-      let filename = WebConsoleUtils.abbreviateSourceURL(sourceURL);
+      let filename = getSourceNames(sourceURL).short;
       let functionName = message.functionName ||
                          l10n.getStr("stacktrace.anonymousFunction");
       a.textContent = l10n.getFormatStr("reflow.messageLinkText",
@@ -2505,12 +2506,11 @@ WebConsoleFrame.prototype = {
       isScratchpad = true;
     } else {
       fullURL = url.split(" -> ").pop();
-      filename = WebConsoleUtils.abbreviateSourceURL(fullURL);
+      filename = getSourceNames(fullURL).short;
     }
 
     filenameNode.className = "filename";
-    filenameNode.textContent =
-      " " + (filename || l10n.getStr("unknownLocation"));
+    filenameNode.textContent = ` ${filename}`;
     locationNode.appendChild(filenameNode);
 
     locationNode.href = isScratchpad || !fullURL ? "#" : fullURL;

devtools/shared/webconsole/utils.js

@@ -180,58 +180,6 @@ var WebConsoleUtils = {
            getInterface(Ci.nsIDOMWindowUtils).outerWindowID;
   },
 
-  /**
-   * Abbreviates the given source URL so that it can be displayed flush-right
-   * without being too distracting.
-   *
-   * @param string aSourceURL
-   *        The source URL to shorten.
-   * @param object [aOptions]
-   *        Options:
-   *        - onlyCropQuery: boolean that tells if the URL abbreviation function
-   *        should only remove the query parameters and the hash fragment from
-   *        the given URL.
-   * @return string
-   *         The abbreviated form of the source URL.
-   */
-  abbreviateSourceURL:
-  function WCU_abbreviateSourceURL(aSourceURL, aOptions = {})
-  {
-    if (!aOptions.onlyCropQuery && aSourceURL.substr(0, 5) == "data:") {
-      let commaIndex = aSourceURL.indexOf(",");
-      if (commaIndex > -1) {
-        aSourceURL = "data:" + aSourceURL.substring(commaIndex + 1);
-      }
-    }
-
-    // Remove any query parameters.
-    let hookIndex = aSourceURL.indexOf("?");
-    if (hookIndex > -1) {
-      aSourceURL = aSourceURL.substring(0, hookIndex);
-    }
-
-    // Remove any hash fragments.
-    let hashIndex = aSourceURL.indexOf("#");
-    if (hashIndex > -1) {
-      aSourceURL = aSourceURL.substring(0, hashIndex);
-    }
-
-    // Remove a trailing "/".
-    if (aSourceURL[aSourceURL.length - 1] == "/") {
-      aSourceURL = aSourceURL.replace(/\/+$/, "");
-    }
-
-    // Remove all but the last path component.
-    if (!aOptions.onlyCropQuery) {
-      let slashIndex = aSourceURL.lastIndexOf("/");
-      if (slashIndex > -1) {
-        aSourceURL = aSourceURL.substring(slashIndex + 1);
-      }
-    }
-
-    return aSourceURL;
-  },
-
   /**
    * Tells if the given function is native or not.
    *