From cff86946bf3a7e84eff093f040d6d60805860e86 Mon Sep 17 00:00:00 2001 From: Jordan Santell Date: Fri, 26 Feb 2016 09:36:43 -0800 Subject: [PATCH] Bug 1251084 - Remove WebConsoleUtils.abbreviateSourceURL with source-utils function. r=linclark --- .../locales/en-US/webconsole.properties | 5 - .../memory/components/shortest-paths.js | 7 +- .../performance/modules/logic/marker-utils.js | 4 +- devtools/client/shared/components/frame.js | 11 +-- devtools/client/shared/source-utils.js | 70 ++++++++++++-- .../shared/test/unit/test_source-utils.js | 94 ++++++++++++++++--- .../client/shared/widgets/VariablesView.jsm | 8 +- devtools/client/webconsole/console-output.js | 5 +- devtools/client/webconsole/test/browser.ini | 1 - ...rowser_webconsole_abbreviate_source_url.js | 25 ----- .../test-console-output-dom-elements.html | 12 ++- devtools/client/webconsole/webconsole.js | 8 +- devtools/shared/webconsole/utils.js | 52 ---------- 13 files changed, 169 insertions(+), 133 deletions(-) delete mode 100644 devtools/client/webconsole/test/browser_webconsole_abbreviate_source_url.js diff --git a/devtools/client/locales/en-US/webconsole.properties b/devtools/client/locales/en-US/webconsole.properties index 13448d21426..17ace43a74b 100644 --- a/devtools/client/locales/en-US/webconsole.properties +++ b/devtools/client/locales/en-US/webconsole.properties @@ -98,11 +98,6 @@ stacktrace.anonymousFunction= # %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= - # 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 diff --git a/devtools/client/memory/components/shortest-paths.js b/devtools/client/memory/components/shortest-paths.js index 4b154f73378..ee984cee5e9 100644 --- a/devtools/client/memory/components/shortest-paths.js +++ b/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"); diff --git a/devtools/client/performance/modules/logic/marker-utils.js b/devtools/client/performance/modules/logic/marker-utils.js index 0edf5c292ae..b2a67bc62f4 100644 --- a/devtools/client/performance/modules/logic/marker-utils.js +++ b/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}`); diff --git a/devtools/client/shared/components/frame.js b/devtools/client/shared/components/frame.js index b3d3e5f0a7c..334689df8ff 100644 --- a/devtools/client/shared/components/frame.js +++ b/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({ diff --git a/devtools/client/shared/source-utils.js b/devtools/client/shared/source-utils.js index 740116852c1..575dcbe7103 100644 --- a/devtools/client/shared/source-utils.js +++ b/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; diff --git a/devtools/client/shared/test/unit/test_source-utils.js b/devtools/client/shared/test/unit/test_source-utils.js index ea92ce3d159..c31af830b50 100644 --- a/devtools/client/shared/test/unit/test_source-utils.js +++ b/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,"; + 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("", ""); - equal(results.short, ""); - equal(results.long, ""); - 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,", + "data:", + "data:text/html;charset=utf-8,"); + + 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`); + } }); diff --git a/devtools/client/shared/widgets/VariablesView.jsm b/devtools/client/shared/widgets/VariablesView.jsm index 7dfdafe0cc1..e8b79ac5681 100644 --- a/devtools/client/shared/widgets/VariablesView.jsm +++ b/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; diff --git a/devtools/client/webconsole/console-output.js b/devtools/client/webconsole/console-output.js index 9b8c90f6d13..d1318e1ff31 100644 --- a/devtools/client/webconsole/console-output.js +++ b/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 }); } diff --git a/devtools/client/webconsole/test/browser.ini b/devtools/client/webconsole/test/browser.ini index 4963863309c..58efd929b87 100644 --- a/devtools/client/webconsole/test/browser.ini +++ b/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] diff --git a/devtools/client/webconsole/test/browser_webconsole_abbreviate_source_url.js b/devtools/client/webconsole/test/browser_webconsole_abbreviate_source_url.js deleted file mode 100644 index 67a49fba1e7..00000000000 --- a/devtools/client/webconsole/test/browser_webconsole_abbreviate_source_url.js +++ /dev/null @@ -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); -} - diff --git a/devtools/client/webconsole/test/test-console-output-dom-elements.html b/devtools/client/webconsole/test/test-console-output-dom-elements.html index c0605ae3c4e..8046e59185d 100644 --- a/devtools/client/webconsole/test/test-console-output-dom-elements.html +++ b/devtools/client/webconsole/test/test-console-output-dom-elements.html @@ -2,7 +2,7 @@ - Test the web console output - 05 + Test the web console output - dom elements +