Closed Bug 642419 Opened 12 years ago Closed 12 years ago
Errors in content scripts don't get displayed anywhere
Currently if you have an error in a content script, you get no notification of it. Nothing in the error console, nothing on the console in a debug build (IIRC), your code just doesn't work. This makes touching the SpecialPowers code extremely frustrating, as it's very easy to make a syntax error and then the tests just fail with "SpecialPowers is not defined". We should forward message manager script errors to the error console. (Ideally that would work in the OOP scenario as well, but I'd settle for it working in-process to start.)
I looked for a dupe, because I swore I filed this bug the last time I hit it, but I couldn't find one.
I suspect the problem is that nsFrameMessageManager::LoadFrameScriptInternal isn't reporting any kind of failure if JS_ExecuteScriptInternal returns false: >631 JS_ExecuteScript(mCx, global, >632 (JSScript*)JS_GetPrivate(mCx, holder->mObject), >633 &val); >706 JS_ExecuteScript(mCx, global, >707 (JSScript*)JS_GetPrivate(mCx, scriptObj), &val);
Hmm, I'm at least partially wrong. I still suspect that we need to call JS_ReportPendingError if those JS_ExecuteScript calls fail, but a flat-out syntax error in a content script is causing JS_CompileUCScriptForPrincipals to return a null script object. I don't see any built-in way that that gets reported (since we can end up loading lots and lots of frame scripts at weird points from one call to LoadFrameScript, returning non-NS_OK doesn't seem right), so I guess I need to manually report the error.
Good, my hunch is validated. Calling an undefined function in a content script results in JS_ExecuteScript returning false, but strangely there doesn't seem to be an exception pending.
For those following along at home, I've now learned about JS_SetErrorReporter and how the message manager doesn't call it. I'm going to throw together an error reporter and see what happens.
With this patch, mochitests still only show a generic "SpecialPowers is not defined". However, if you look in the error console you'll see errors with links pointing to the culprit content script. I'm not quite sure how the mochitest errors are reported but I suspect it's with window.onerror, which doesn't seem like the correct place for these errors to be showing up.
The error console would be absolutely perfect. That's where I was looking for errors (and not finding them). Plus, I think we echo that text to stdout or stderr in debug builds, right?
Comment on attachment 520125 [details] [diff] [review] Add error reporting for content scripts. why the (void) in few places? Does some compiler give warnings without those? I wish we could reuse NS_ScriptErrorReporter, but that is way too DOM-bound atm :(
Attachment #520125 - Flags: review?(Olli.Pettay) → review+
I added the (void)s because I want to show that we are explicitly choosing to do nothing special when the scripts fail to execute.
Hmm, forgot about this. Let's push it.
patching file content/base/src/nsFrameMessageManager.cpp Hunk #3 FAILED at 671 Hunk #4 FAILED at 746
Status: NEW → ASSIGNED
The two hunks that fail only prefix the "(void)" return type handler to the existing code. The previous code was changed by: http://hg.mozilla.org/mozilla-central/diff/300e998d0291/content/base/src/nsFrameMessageManager.cpp So I just added the "(void)" prefix to the new code. Testing
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.