Bug 1493361 - Use StringRep to render PageError message. r=Honza.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Mon, 13 May 2019 12:58:33 +0000
changeset 535491 b87cdb4550833374f77f1bc5163e554552fb35f9
parent 535490 395e27a912b3b6aea3c27ccdb8a0028c23abf223
child 535492 36629188da45f0fd85746523f7986621839c6f33
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1493361
milestone68.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 1493361 - Use StringRep to render PageError message. r=Honza. This allows us to benefit from the linkification that is done there. We crop the URL at 120 chars for now. We might consider bumping this up depending on the feedback we get. We add both a mocha and a mochitest to make sure this work as expected. Differential Revision: https://phabricator.services.mozilla.com/D29007
devtools/client/webconsole/components/message-types/PageError.js
devtools/client/webconsole/test/components/page-error.test.js
devtools/client/webconsole/test/mochitest/browser.ini
devtools/client/webconsole/test/mochitest/browser_webconsole_error_with_url.js
--- a/devtools/client/webconsole/components/message-types/PageError.js
+++ b/devtools/client/webconsole/components/message-types/PageError.js
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 // React & Redux
 const { createFactory } = require("devtools/client/shared/vendor/react");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const Message = createFactory(require("devtools/client/webconsole/components/Message"));
+const { MODE, REPS } = require("devtools/client/shared/components/reps/reps");
 
 PageError.displayName = "PageError";
 
 PageError.propTypes = {
   message: PropTypes.object.isRequired,
   open: PropTypes.bool,
   timestampsVisible: PropTypes.bool.isRequired,
   serviceContainer: PropTypes.object,
@@ -47,22 +48,24 @@ function PageError(props) {
     messageText,
     stacktrace,
     frame,
     exceptionDocURL,
     timeStamp,
     notes,
   } = message;
 
-  let messageBody;
-  if (typeof messageText === "string") {
-    messageBody = messageText;
-  } else if (typeof messageText === "object" && messageText.type === "longString") {
-    messageBody = `${message.messageText.initial}…`;
-  }
+  const messageBody = REPS.StringRep.rep({
+    object: messageText,
+    mode: MODE.LONG,
+    useQuotes: false,
+    escapeWhitespace: false,
+    urlCropLimit: 120,
+    openLink: serviceContainer.openLink,
+  });
 
   return Message({
     dispatch,
     messageId,
     executionPoint,
     isPaused,
     open,
     collapsible: Array.isArray(stacktrace),
--- a/devtools/client/webconsole/test/components/page-error.test.js
+++ b/devtools/client/webconsole/test/components/page-error.test.js
@@ -6,27 +6,29 @@
 const expect = require("expect");
 const { render, mount } = require("enzyme");
 const sinon = require("sinon");
 
 // React
 const { createFactory } = require("devtools/client/shared/vendor/react");
 const Provider = createFactory(require("react-redux").Provider);
 const { setupStore } = require("devtools/client/webconsole/test/helpers");
+const { prepareMessage } = require("devtools/client/webconsole/utils/messages");
 
 // Components under test.
 const PageError = require("devtools/client/webconsole/components/message-types/PageError");
 const {
   MESSAGE_OPEN,
   MESSAGE_CLOSE,
 } = require("devtools/client/webconsole/constants");
 const { INDENT_WIDTH } = require("devtools/client/webconsole/components/MessageIndent");
 
 // Test fakes.
-const { stubPreparedMessages } = require("devtools/client/webconsole/test/fixtures/stubs/index");
+const { stubPackets, stubPreparedMessages } =
+  require("devtools/client/webconsole/test/fixtures/stubs/index");
 const serviceContainer = require("devtools/client/webconsole/test/fixtures/serviceContainer");
 
 describe("PageError component:", () => {
   it("renders", () => {
     const message = stubPreparedMessages.get("ReferenceError: asdf is not defined");
     const wrapper = render(PageError({
       message,
       serviceContainer,
@@ -78,16 +80,61 @@ describe("PageError component:", () => {
 
   it("renders thrown string", () => {
     const message = stubPreparedMessages.get(`throw "tomato"`);
     const wrapper = render(PageError({ message, serviceContainer }));
     const text = wrapper.find(".message-body").text();
     expect(text).toBe(`uncaught exception: tomato`);
   });
 
+  it("renders URLs in message as actual, cropped, links", () => {
+    // Let's replace the packet data in order to mimick a pageError.
+    const packet = stubPackets.get("ReferenceError: asdf is not defined");
+
+    const evilDomain = `https://evil.com/?`;
+    const badDomain = `https://not-so-evil.com/?`;
+    const paramLength = 200;
+    const longParam = "a".repeat(paramLength);
+
+    const evilURL = `${evilDomain}${longParam}`;
+    const badURL = `${badDomain}${longParam}`;
+
+    packet.pageError.errorMessage =
+      `“${evilURL}“ is evil and “${badURL}“ is not good either`;
+
+    // We remove the exceptionDocURL to not have the "learn more" link.
+    packet.pageError.exceptionDocURL = null;
+
+    const message = prepareMessage(packet, {getNextId: () => "1"});
+    const wrapper = render(PageError({ message, serviceContainer }));
+
+    // Keep in sync with `urlCropLimit` in PageError.js.
+    const cropLimit = 120;
+    const partLength = cropLimit / 2;
+    const getCroppedUrl = url =>
+      `${url}${"a".repeat((partLength - url.length))}…${"a".repeat(partLength)}`;
+
+    const croppedEvil = getCroppedUrl(evilDomain);
+    const croppedbad = getCroppedUrl(badDomain);
+
+    const text = wrapper.find(".message-body").text();
+    expect(text).toBe(
+      `“${croppedEvil}“ is evil and “${croppedbad}“ is not good either`);
+
+    // There should be 2 links.
+    const links = wrapper.find(".message-body a");
+    expect(links.length).toBe(2);
+
+    expect(links.eq(0).attr("href")).toBe(evilURL);
+    expect(links.eq(0).attr("title")).toBe(evilURL);
+
+    expect(links.eq(1).attr("href")).toBe(badURL);
+    expect(links.eq(1).attr("title")).toBe(badURL);
+  });
+
   it("displays a [Learn more] link", () => {
     const store = setupStore();
 
     const message = stubPreparedMessages.get("ReferenceError: asdf is not defined");
 
     serviceContainer.openLink = sinon.spy();
     const wrapper = mount(Provider({store},
       PageError({
--- a/devtools/client/webconsole/test/mochitest/browser.ini
+++ b/devtools/client/webconsole/test/mochitest/browser.ini
@@ -291,16 +291,17 @@ tags = clipboard
 [browser_webconsole_csp_ignore_reflected_xss_message.js]
 [browser_webconsole_csp_violation.js]
 [browser_webconsole_cspro.js]
 [browser_webconsole_document_focus.js]
 [browser_webconsole_duplicate_errors.js]
 [browser_webconsole_error_with_grouped_stack.js]
 [browser_webconsole_error_with_longstring_stack.js]
 [browser_webconsole_error_with_unicode.js]
+[browser_webconsole_error_with_url.js]
 [browser_webconsole_errors_after_page_reload.js]
 [browser_webconsole_eval_error.js]
 [browser_webconsole_eval_in_debugger_stackframe.js]
 [browser_webconsole_eval_in_debugger_stackframe2.js]
 [browser_webconsole_eval_sources.js]
 [browser_webconsole_execution_scope.js]
 [browser_webconsole_external_script_errors.js]
 [browser_webconsole_file_uri.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/mochitest/browser_webconsole_error_with_url.js
@@ -0,0 +1,50 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Check if an error with Unicode characters is reported correctly.
+
+"use strict";
+
+const longParam = "0".repeat(200);
+const url1 = `https://example.com?v=${longParam}`;
+const url2 = `https://example.org?v=${longParam}`;
+
+const TEST_URI = `data:text/html;charset=utf8,<script>
+  throw "Visit \u201c${url1}\u201d or \u201c${url2}\u201d to get more " +
+        "information on this error.";
+</script>`;
+const {ELLIPSIS} = require("devtools/shared/l10n");
+
+add_task(async function() {
+  const hud = await openNewTabAndConsole(TEST_URI);
+
+  // On e10s, the exception is triggered in child process
+  // and is ignored by test harness
+  if (!Services.appinfo.browserTabsRemoteAutostart) {
+    expectUncaughtException();
+  }
+
+  const getCroppedUrl = origin => {
+    const cropLimit = 120;
+    const half = cropLimit / 2;
+    const params =
+      `?v=${"0".repeat(half - origin.length - 3)}${ELLIPSIS}${"0".repeat(half)}`;
+    return `${origin}${params}`;
+  };
+
+  const EXPECTED_MESSAGE = `get more information on this error`;
+
+  const msg = await waitFor(() => findMessage(hud, EXPECTED_MESSAGE));
+  ok(msg, `Link in error message are cropped as expected`);
+
+  const [comLink, orgLink] = Array.from(msg.querySelectorAll("a"));
+  is(comLink.getAttribute("href"), url1, "First link has expected url");
+  is(comLink.getAttribute("title"), url1, "First link has expected tooltip");
+  is(comLink.textContent, getCroppedUrl("https://example.com"),
+    "First link has expected text");
+
+  is(orgLink.getAttribute("href"), url2, "Second link has expected url");
+  is(orgLink.getAttribute("title"), url2, "Second link has expected tooltip");
+  is(orgLink.textContent, getCroppedUrl("https://example.org"),
+    "Second link has expected text");
+});