Last Comment Bug 661035 - make errors in scripts compiled by nsFrameMessageManager::LoadFrameScript more visible
: make errors in scripts compiled by nsFrameMessageManager::LoadFrameScript mor...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: IPC (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla8
Assigned To: Josh Matthews [:jdm]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-31 16:13 PDT by Luke Wagner [:luke]
Modified: 2011-07-26 11:06 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Spit out content frame script errors to stdout. (30.24 KB, patch)
2011-07-08 13:14 PDT, Josh Matthews [:jdm]
no flags Details | Diff | Review
Print frame script errors to stdout. (1.64 KB, patch)
2011-07-08 13:16 PDT, Josh Matthews [:jdm]
bugs: review+
Details | Diff | Review

Description Luke Wagner [:luke] 2011-05-31 16:13:44 PDT
Injecting a syntax error in, say, browser.js causes a JS syntax error message to pop out in console spew.  AFAICS, errors in scripts compiled via nsFrameMessageManager::LoadFrameScript issue no such report.  The exception generated by JS does get converted to an error report passed to ContentScriptErrorReporter which passes it on to nsConsoleService::LogMessage.  I'm not sure where in this process useful console output is supposed to be emitted.  Dolske said this has been a common complaint.  I know a visible error report would have personally saved me quite a few hours tracking down a recent bug.
Comment 1 Josh Matthews [:jdm] 2011-05-31 16:17:38 PDT
Ah, the error reporter that I wrote didn't include the console bit from NS_ScriptErrorReporter. Relevant bits of code:
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsFrameMessageManager.cpp#538
http://mxr.mozilla.org/mozilla-central/source/dom/base/nsJSEnvironment.cpp#493
Comment 2 Josh Matthews [:jdm] 2011-07-08 13:14:26 PDT
Created attachment 544887 [details] [diff] [review]
Spit out content frame script errors to stdout.
Comment 3 Josh Matthews [:jdm] 2011-07-08 13:16:46 PDT
Created attachment 544888 [details] [diff] [review]
Print frame script errors to stdout.
Comment 4 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-07-08 13:25:36 PDT
Comment on attachment 544888 [details] [diff] [review]
Print frame script errors to stdout.

># HG changeset patch
># User Josh Matthews <josh@joshmatthews.net>
># Date 1310156158 14400
># Node ID f75a8a9a4723475a5d351e149b402fce06f1050d
># Parent  31e88a02c7d5ffe508de9731368190e1ed9b9ecc
>Bug 661035 - Print frame script errors to stdout. r=smaug
>
>diff --git a/content/base/src/nsFrameMessageManager.cpp b/content/base/src/nsFrameMessageManager.cpp
>--- a/content/base/src/nsFrameMessageManager.cpp
>+++ b/content/base/src/nsFrameMessageManager.cpp
>@@ -578,16 +578,42 @@ ContentScriptErrorReporter(JSContext* aC
>     return;
>   }
> 
>   nsCOMPtr<nsIConsoleService> consoleService =
>       do_GetService(NS_CONSOLESERVICE_CONTRACTID);
>   if (consoleService) {
>     (void) consoleService->LogMessage(scriptError);
>   }
>+
>+#ifdef DEBUG
>+  // Print it to stderr as well, for the benefit of those invoking
>+  // mozilla with -console.
>+  nsCAutoString error;
>+  error.Assign("JavaScript ");
>+  if (JSREPORT_IS_STRICT(flags))
>+    error.Append("strict ");
if (expr) {
  stmt;
}

>+  if (JSREPORT_IS_WARNING(flags))
>+    error.Append("warning: ");
>+  else
>+    error.Append("error: ");

if (expr) {
  stmt;
} else {
  stmt;
}
Comment 6 :Ehsan Akhgari (out sick) 2011-07-26 11:06:18 PDT
http://hg.mozilla.org/mozilla-central/rev/820b80945342

Note You need to log in before you can comment on or make changes to this bug.