Bug 1255133 - Surface links to external documentation alongside relevant error messages 1/2; r=bgrins
authorMorgan Phillips <winter2718@gmail.com>
Fri, 01 Apr 2016 23:17:49 -0700
changeset 291430 873bdc9379fda4887e4a19bfc82998276e1a65d4
parent 291429 9adc763aae7608252bb3e9df1ef7888da4f9f2ba
child 291431 8e7f918c59ea2c82e3a72ce0aac3296e507983ce
push id30134
push userryanvm@gmail.com
push dateMon, 04 Apr 2016 01:40:30 +0000
treeherdermozilla-central@cfd51e67b26e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1255133
milestone48.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 1255133 - Surface links to external documentation alongside relevant error messages 1/2; r=bgrins
devtools/client/webconsole/test/browser.ini
devtools/client/webconsole/test/browser_webconsole_script_errordoc_urls.js
devtools/client/webconsole/webconsole.js
devtools/server/actors/errordocs.js
devtools/server/actors/moz.build
devtools/server/actors/webconsole.js
devtools/shared/webconsole/test/test_page_errors.html
js/xpconnect/idl/nsIScriptError.idl
js/xpconnect/src/nsScriptError.cpp
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/src/xpcprivate.h
js/xpconnect/src/xpcpublic.h
--- a/devtools/client/webconsole/test/browser.ini
+++ b/devtools/client/webconsole/test/browser.ini
@@ -188,16 +188,18 @@ skip-if = e10s && debug && (os == 'win' 
 [browser_longstring_hang.js]
 [browser_output_breaks_after_console_dir_uninspectable.js]
 [browser_output_longstring_expand.js]
 [browser_repeated_messages_accuracy.js]
 [browser_result_format_as_string.js]
 [browser_warn_user_about_replaced_api.js]
 [browser_webconsole_allow_mixedcontent_securityerrors.js]
 tags = mcb
+[browser_webconsole_script_errordoc_urls.js]
+skip-if = e10s # Bug 1042253 - webconsole tests disabled with e10s
 [browser_webconsole_assert.js]
 [browser_webconsole_block_mixedcontent_securityerrors.js]
 tags = mcb
 [browser_webconsole_bug_579412_input_focus.js]
 [browser_webconsole_bug_580001_closing_after_completion.js]
 [browser_webconsole_bug_580030_errors_after_page_reload.js]
 [browser_webconsole_bug_580454_timestamp_l10n.js]
 [browser_webconsole_bug_582201_duplicate_errors.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/browser_webconsole_script_errordoc_urls.js
@@ -0,0 +1,67 @@
+/* -*- 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/ */
+
+// Ensure that [Learn More] links appear alongside any errors listed
+// in "errordocs.js". Note: this only tests script execution.
+
+"use strict";
+
+const ErrorDocs = require("devtools/server/actors/errordocs");
+
+function makeURIData(script) {
+  return `data:text/html;charset=utf8,<script>${script}</script>`
+}
+
+const TestData = [
+  {
+    jsmsg: "JSMSG_READ_ONLY",
+    script: "'use strict'; (Object.freeze({name: 'Elsa', score: 157})).score = 0;",
+    isException: true,
+  },
+  {
+    jsmsg: "JSMSG_STMT_AFTER_RETURN",
+    script: "function a() { return; 1 + 1; };",
+    isException: false,
+  }
+]
+
+add_task(function* () {
+  yield loadTab("data:text/html;charset=utf8,errordoc tests");
+
+  let hud = yield openConsole();
+
+  for (let i = 0; i < TestData.length; i++) {
+    yield testScriptError(hud, TestData[i]);
+  }
+});
+
+function* testScriptError(hud, testData) {
+  if (testData.isException === true) {
+    expectUncaughtException();
+  }
+
+  BrowserTestUtils.loadURI(gBrowser.selectedBrowser, makeURIData(testData.script));
+
+  yield waitForMessages({
+    webconsole: hud,
+    messages: [
+      {
+        category: CATEGORY_JS
+      }
+    ]
+  });
+
+  // grab the most current error doc URL
+  let url = ErrorDocs.GetURL(testData.jsmsg);
+
+  let hrefs = {};
+  for (let link of hud.jsterm.outputNode.querySelectorAll("a")) {
+    hrefs[link.href] = true;
+  }
+
+  ok(url in hrefs, `Expected a link to ${url}.`);
+
+  hud.jsterm.clearOutput();
+}
--- a/devtools/client/webconsole/webconsole.js
+++ b/devtools/client/webconsole/webconsole.js
@@ -11,16 +11,17 @@ 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 BrowserLoaderModule = {};
 Cu.import("resource://devtools/client/shared/browser-loader.js", BrowserLoaderModule);
 
 const promise = require("promise");
 const Services = require("Services");
+const ErrorDocs = require("devtools/server/actors/errordocs");
 
 loader.lazyServiceGetter(this, "clipboardHelper",
                          "@mozilla.org/widget/clipboardhelper;1",
                          "nsIClipboardHelper");
 loader.lazyRequireGetter(this, "EventEmitter", "devtools/shared/event-emitter");
 loader.lazyRequireGetter(this, "AutocompletePopup", "devtools/client/shared/autocomplete-popup", true);
 loader.lazyRequireGetter(this, "ToolSidebar", "devtools/client/framework/sidebar", true);
 loader.lazyRequireGetter(this, "ConsoleOutput", "devtools/client/webconsole/console-output", true);
@@ -1484,16 +1485,17 @@ WebConsoleFrame.prototype = {
       private: scriptError.private,
       filterDuplicates: true
     });
 
     let node = msg.init(this.output).render().element;
 
     // Select the body of the message node that is displayed in the console
     let msgBody = node.getElementsByClassName("message-body")[0];
+
     // Add the more info link node to messages that belong to certain categories
     this.addMoreInfoLink(msgBody, scriptError);
 
     if (objectActors.size > 0) {
       node._objectActors = objectActors;
     }
 
     return node;
@@ -1692,21 +1694,24 @@ WebConsoleFrame.prototype = {
         break;
       case "SHA-1 Signature":
         url = WEAK_SIGNATURE_ALGORITHM_LEARN_MORE;
         break;
       case "Tracking Protection":
         url = TRACKING_PROTECTION_LEARN_MORE;
         break;
       default:
-        // Unknown category. Return without adding more info node.
-        return;
+        // If all else fails check for an error doc URL.
+        url = ErrorDocs.GetURL(scriptError.errorMessageName);
+        break;
     }
 
-    this.addLearnMoreWarningNode(node, url);
+    if (url) {
+      this.addLearnMoreWarningNode(node, url);
+    }
   },
 
   /*
    * Appends a clickable warning node to the node passed
    * as a parameter to the function. When a user clicks on the appended
    * warning node, the browser navigates to the provided url.
    *
    * @param node
new file mode 100644
--- /dev/null
+++ b/devtools/server/actors/errordocs.js
@@ -0,0 +1,23 @@
+/* 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/. */
+
+/**
+ * A mapping of error message names to external documentation. Any error message
+ * included here will be displayed alongside its link in the web console.
+ */
+
+"use strict";
+
+const ErrorDocs = {
+    JSMSG_READ_ONLY: "https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Read-only",
+    JSMSG_BAD_ARRAY_LENGTH: "https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Invalid_array_length",
+    JSMSG_NEGATIVE_REPETITION_COUNT: "https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Negative_repetition_count",
+    JSMSG_RESULTING_STRING_TOO_LARGE: "https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Resulting_string_too_large",
+    JSMSG_BAD_RADIX: "https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Bad_radix",
+    JSMSG_PRECISION_RANGE: "https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Precision_range",
+    JSMSG_BAD_FORMAL: "https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Malformed_formal_parameter",
+    JSMSG_STMT_AFTER_RETURN: "https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Stmt_after_return",
+}
+
+exports.GetURL = (errorName) => ErrorDocs[errorName];
--- a/devtools/server/actors/moz.build
+++ b/devtools/server/actors/moz.build
@@ -20,16 +20,17 @@ DevToolsModules(
     'childtab.js',
     'chrome.js',
     'common.js',
     'csscoverage.js',
     'device.js',
     'director-manager.js',
     'director-registry.js',
     'environment.js',
+    'errordocs.js',
     'eventlooplag.js',
     'frame.js',
     'framerate.js',
     'gcli.js',
     'heap-snapshot-file.js',
     'highlighters.css',
     'highlighters.js',
     'inspector.js',
--- a/devtools/server/actors/webconsole.js
+++ b/devtools/server/actors/webconsole.js
@@ -1387,16 +1387,17 @@ WebConsoleActor.prototype =
     }
     let lineText = aPageError.sourceLine;
     if (lineText && lineText.length > DebuggerServer.LONG_STRING_INITIAL_LENGTH) {
       lineText = lineText.substr(0, DebuggerServer.LONG_STRING_INITIAL_LENGTH);
     }
 
     return {
       errorMessage: this._createStringGrip(aPageError.errorMessage),
+      errorMessageName: aPageError.errorMessageName,
       sourceName: aPageError.sourceName,
       lineText: lineText,
       lineNumber: aPageError.lineNumber,
       columnNumber: aPageError.columnNumber,
       category: aPageError.category,
       timeStamp: aPageError.timeStamp,
       warning: !!(aPageError.flags & aPageError.warningFlag),
       error: !!(aPageError.flags & aPageError.errorFlag),
--- a/devtools/shared/webconsole/test/test_page_errors.html
+++ b/devtools/shared/webconsole/test/test_page_errors.html
@@ -13,50 +13,134 @@
 
 <script class="testbody" type="text/javascript;version=1.8">
 SimpleTest.waitForExplicitFinish();
 
 let expectedPageErrors = [];
 
 function doPageErrors()
 {
-  expectedPageErrors = [
-    {
+  expectedPageErrors = {
+    "document.body.style.color = 'fooColor';": {
       errorMessage: /fooColor/,
+      errorMessageName: undefined,
       sourceName: /test_page_errors/,
       category: "CSS Parser",
       timeStamp: /^\d+$/,
       error: false,
       warning: true,
       exception: false,
       strict: false,
     },
-    {
+   "document.doTheImpossible();": {
       errorMessage: /doTheImpossible/,
+      errorMessageName: undefined,
+      sourceName: /test_page_errors/,
+      category: "chrome javascript",
+      timeStamp: /^\d+$/,
+      error: false,
+      warning: false,
+      exception: true,
+      strict: false,
+    },
+    "(42).toString(0);": {
+      errorMessage: /radix/,
+      errorMessageName: "JSMSG_BAD_RADIX",
       sourceName: /test_page_errors/,
       category: "chrome javascript",
       timeStamp: /^\d+$/,
       error: false,
       warning: false,
       exception: true,
       strict: false,
     },
-  ];
+     "'use strict'; (Object.freeze({name: 'Elsa', score: 157})).score = 0;": {
+      errorMessage: /read.only/,
+      errorMessageName: "JSMSG_READ_ONLY",
+      sourceName: /test_page_errors/,
+      category: "chrome javascript",
+      timeStamp: /^\d+$/,
+      error: false,
+      warning: false,
+      exception: true,
+    },
+     "([]).length = -1": {
+      errorMessage: /array length/,
+      errorMessageName: "JSMSG_BAD_ARRAY_LENGTH",
+      sourceName: /test_page_errors/,
+      category: "chrome javascript",
+      timeStamp: /^\d+$/,
+      error: false,
+      warning: false,
+      exception: true,
+    },
+     "'abc'.repeat(-1);": {
+      errorMessage: /repeat count.*non-negative/,
+      errorMessageName: "JSMSG_NEGATIVE_REPETITION_COUNT",
+      sourceName: /test_page_errors/,
+      category: "chrome javascript",
+      timeStamp: /^\d+$/,
+      error: false,
+      warning: false,
+      exception: true,
+    },
+    "'a'.repeat(2**28);": {
+      errorMessage: /repeat count.*less than infinity/,
+      errorMessageName: "JSMSG_RESULTING_STRING_TOO_LARGE",
+      sourceName: /test_page_errors/,
+      category: "chrome javascript",
+      timeStamp: /^\d+$/,
+      error: false,
+      warning: false,
+      exception: true,
+    },
+    "77.1234.toExponential(-1);": {
+      errorMessage: /out of range/,
+      errorMessageName: "JSMSG_PRECISION_RANGE",
+      sourceName: /test_page_errors/,
+      category: "chrome javascript",
+      timeStamp: /^\d+$/,
+      error: false,
+      warning: false,
+      exception: true,
+    },
+    "var f = Function('x y', 'return x + y;');": {
+      errorMessage: /malformed formal/,
+      errorMessageName: "JSMSG_BAD_FORMAL",
+      sourceName: /test_page_errors/,
+      category: "chrome javascript",
+      timeStamp: /^\d+$/,
+      error: false,
+      warning: false,
+      exception: true,
+    },
+    "function a() { return; 1 + 1; }": {
+      errorMessage: /unreachable code/,
+      errorMessageName: "JSMSG_STMT_AFTER_RETURN",
+      sourceName: /test_page_errors/,
+      category: "chrome javascript",
+      timeStamp: /^\d+$/,
+      error: false,
+      warning: true,
+      exception: false,
+    },
+  };
 
   let container = document.createElement("script");
-  document.body.appendChild(container);
-  container.textContent = "document.body.style.color = 'fooColor';";
-  document.body.removeChild(container);
-
-  SimpleTest.expectUncaughtException();
-
-  container = document.createElement("script");
-  document.body.appendChild(container);
-  container.textContent = "document.doTheImpossible();";
-  document.body.removeChild(container);
+  for (let stmt of Object.keys(expectedPageErrors)) {
+      if (expectedPageErrors[stmt].exception) {
+          SimpleTest.expectUncaughtException();
+      }
+      info("starting stmt: " + stmt);
+      container = document.createElement("script");
+      document.body.appendChild(container);
+      container.textContent = stmt;
+      document.body.removeChild(container);
+      info("ending stmt: " + stmt);
+  }
 }
 
 function startTest()
 {
   removeEventListener("load", startTest);
 
   attachConsole(["PageError"], onAttach);
 }
@@ -75,25 +159,25 @@ function onPageError(aState, aType, aPac
   if (!aPacket.pageError.sourceName.includes("test_page_errors")) {
     info("Ignoring error from unknown source: " + aPacket.pageError.sourceName);
     return;
   }
 
   is(aPacket.from, aState.actor, "page error actor");
 
   pageErrors.push(aPacket.pageError);
-  if (pageErrors.length != expectedPageErrors.length) {
+  if (pageErrors.length != Object.keys(expectedPageErrors).length) {
     return;
   }
 
   aState.dbgClient.removeListener("pageError", onPageError);
 
-  expectedPageErrors.forEach(function(aMessage, aIndex) {
+  Object.values(expectedPageErrors).forEach(function(aMessage, aIndex) {
     info("checking received page error #" + aIndex);
-    checkObject(pageErrors[aIndex], expectedPageErrors[aIndex]);
+    checkObject(pageErrors[aIndex], Object.values(expectedPageErrors)[aIndex]);
   });
 
   closeDebugger(aState, function() {
     SimpleTest.finish();
   });
 }
 
 addEventListener("load", startTest);
--- a/js/xpconnect/idl/nsIScriptError.idl
+++ b/js/xpconnect/idl/nsIScriptError.idl
@@ -10,17 +10,17 @@
 
 #include "nsISupports.idl"
 #include "nsIConsoleMessage.idl"
 
 %{C++
 #include "nsStringGlue.h" // for nsDependentCString
 %}
 
-[scriptable, uuid(18bdefde-e57b-11e4-832a-000c29a57fff)]
+[scriptable, uuid(361be358-76f0-47aa-b37b-6ad833599e8d)]
 interface nsIScriptError : nsIConsoleMessage
 {
     /** pseudo-flag for default case */
     const unsigned long errorFlag = 0x0;
 
     /** message is warning */
     const unsigned long warningFlag = 0x1;
 
@@ -63,16 +63,23 @@ interface nsIScriptError : nsIConsoleMes
     /* Get the inner window id this was initialized with.  Zero will be
        returned if init() was used instead of initWithWindowID(). */
     readonly attribute unsigned long long innerWindowID;
 
     readonly attribute boolean isFromPrivateWindow;
 
     attribute jsval stack;
 
+    /**
+     * The name of a template string, as found in js.msg, associated with the
+     * error message.
+     */
+    attribute AString errorMessageName;
+
+
     void init(in AString message,
               in AString sourceName,
               in AString sourceLine,
               in uint32_t lineNumber,
               in uint32_t columnNumber,
               in uint32_t flags,
               in string category);
 
--- a/js/xpconnect/src/nsScriptError.cpp
+++ b/js/xpconnect/src/nsScriptError.cpp
@@ -16,16 +16,17 @@
 #include "nsGlobalWindow.h"
 #include "nsPIDOMWindow.h"
 #include "nsILoadContext.h"
 #include "nsIDocShell.h"
 #include "nsISensitiveInfoHiddenURI.h"
 
 nsScriptErrorBase::nsScriptErrorBase()
     :  mMessage(),
+       mMessageName(),
        mSourceName(),
        mLineNumber(0),
        mSourceLine(),
        mColumnNumber(0),
        mFlags(0),
        mCategory(),
        mOuterWindowID(0),
        mInnerWindowID(0),
@@ -148,16 +149,28 @@ nsScriptErrorBase::GetStack(JS::MutableH
 }
 
 NS_IMETHODIMP
 nsScriptErrorBase::SetStack(JS::HandleValue aStack) {
     return NS_OK;
 }
 
 NS_IMETHODIMP
+nsScriptErrorBase::GetErrorMessageName(nsAString& aErrorMessageName) {
+    aErrorMessageName = mMessageName;
+    return NS_OK;
+}
+
+NS_IMETHODIMP
+nsScriptErrorBase::SetErrorMessageName(const nsAString& aErrorMessageName) {
+    mMessageName = aErrorMessageName;
+    return NS_OK;
+}
+
+NS_IMETHODIMP
 nsScriptErrorBase::Init(const nsAString& message,
                         const nsAString& sourceName,
                         const nsAString& sourceLine,
                         uint32_t lineNumber,
                         uint32_t columnNumber,
                         uint32_t flags,
                         const char* category)
 {
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -26,16 +26,18 @@
 
 #include "nsDOMMutationObserver.h"
 #include "nsICycleCollectorListener.h"
 #include "mozilla/XPTInterfaceInfoManager.h"
 #include "nsIObjectInputStream.h"
 #include "nsIObjectOutputStream.h"
 #include "nsScriptSecurityManager.h"
 
+#include "jsfriendapi.h"
+
 using namespace mozilla;
 using namespace mozilla::dom;
 using namespace xpc;
 using namespace JS;
 
 NS_IMPL_ISUPPORTS(nsXPConnect, nsIXPConnect)
 
 nsXPConnect* nsXPConnect::gSelf = nullptr;
@@ -173,28 +175,34 @@ void
 xpc::ErrorReport::Init(JSErrorReport* aReport, const char* aFallbackMessage,
                        bool aIsChrome, uint64_t aWindowID)
 {
     mCategory = aIsChrome ? NS_LITERAL_CSTRING("chrome javascript")
                           : NS_LITERAL_CSTRING("content javascript");
     mWindowID = aWindowID;
 
     ErrorReportToMessageString(aReport, mErrorMsg);
-
     if (mErrorMsg.IsEmpty() && aFallbackMessage) {
         mErrorMsg.AssignWithConversion(aFallbackMessage);
     }
 
     if (!aReport->filename) {
         mFileName.SetIsVoid(true);
     } else {
         mFileName.AssignWithConversion(aReport->filename);
     }
 
     mSourceLine.Assign(aReport->linebuf(), aReport->linebufLength());
+    const JSErrorFormatString* efs = js::GetErrorMessage(nullptr, aReport->errorNumber);
+
+    if (efs == nullptr) {
+        mErrorMsgName.AssignASCII("");
+    } else {
+        mErrorMsgName.AssignASCII(efs->name);
+    }
 
     mLineNumber = aReport->lineno;
     mColumn = aReport->column;
     mFlags = aReport->flags;
     mIsMuted = aReport->isMuted;
 }
 
 static LazyLogModule gJSDiagnostics("JSDiagnostics");
@@ -243,16 +251,17 @@ xpc::ErrorReport::LogToConsoleWithStack(
       // Only set stack on messages related to a document
       // As we cache messages in the console service,
       // we have to ensure not leaking them after the related
       // context is destroyed and we only track document lifecycle for now.
       errorObject = new nsScriptErrorWithStack(aStack);
     } else {
       errorObject = new nsScriptError();
     }
+    errorObject->SetErrorMessageName(mErrorMsgName);
     NS_ENSURE_TRUE_VOID(consoleService && errorObject);
 
     nsresult rv = errorObject->InitWithWindowID(mErrorMsg, mFileName, mSourceLine,
                                                 mLineNumber, mColumn, mFlags,
                                                 mCategory, mWindowID);
     NS_ENSURE_SUCCESS_VOID(rv);
     consoleService->LogMessage(errorObject);
 
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -2938,16 +2938,17 @@ public:
 
 protected:
     virtual ~nsScriptErrorBase();
 
     void
     InitializeOnMainThread();
 
     nsString mMessage;
+    nsString mMessageName;
     nsString mSourceName;
     uint32_t mLineNumber;
     nsString mSourceLine;
     uint32_t mColumnNumber;
     uint32_t mFlags;
     nsCString mCategory;
     // mOuterWindowID is set on the main thread from InitializeOnMainThread().
     uint64_t mOuterWindowID;
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -515,16 +515,17 @@ class ErrorReport {
     // that this may produce an empty string if aReport doesn't have a
     // message attached.
     static void ErrorReportToMessageString(JSErrorReport* aReport,
                                            nsAString& aString);
 
   public:
 
     nsCString mCategory;
+    nsString mErrorMsgName;
     nsString mErrorMsg;
     nsString mFileName;
     nsString mSourceLine;
     uint64_t mWindowID;
     uint32_t mLineNumber;
     uint32_t mColumn;
     uint32_t mFlags;
     bool mIsMuted;