JS method parsing failure is swallowed

RESOLVED FIXED in mozilla25

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: evilpie, Assigned: bholley)

Tracking

unspecified
mozilla25
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
When this code (http://dxr.mozilla.org/mozilla-central/source/content/xbl/src/nsXBLProtoImplMethod.cpp#l210) fails, we basically swallow the exception from what I have seen. And if the member was in some important xul file like tabbrowser.xml or browser.xml, you can't event debug this. We should print a message to the console, with the name, file and line when this problem arises.
(Reporter)

Updated

5 years ago
Blocks: 895548
(Reporter)

Comment 1

5 years ago
Created attachment 782217 [details] [diff] [review]
xbl-compile-error

This is more of a wallpaper patch that explicitly dumps something. I could imagine that a real solution would try to always report JS exceptions. A hunch tells me that the dummy context stuff created for compiling xul files are not wired up correctly to the error reporting.
Bobby, thoughts?  I thought we had an XBL error reporter for this stuff...
Flags: needinfo?(bobbyholley+bmo)
(Assignee)

Comment 3

5 years ago
Yeah, I'd think we should end up here:

http://mxr.mozilla.org/mozilla-central/source/content/xbl/src/nsXBLDocumentInfo.cpp#214

I couldn't say why we're not without debugging. Tom, do you have simple STR?
Flags: needinfo?(bobbyholley+bmo)
(Reporter)

Comment 4

5 years ago
Mhm I this supposed to be visible in the console? My test case was to add something that doesn't parse to "reload" in browser.xml. This pretty much makes the browser unusable.
(Assignee)

Comment 5

5 years ago
Created attachment 783278 [details] [diff] [review]
Use the system error reporter for XBL compilation. v1

The current XBL error reporter logs things to the console service, but not to
stderr. And in certain situations (especially if there's a parse error in
frontend XBL that leaves the browser in a half-baked state), it can be very
difficult to get to the error console.

So we should log to stderr too, which is exactly what the system error reporter
does.
Attachment #783278 - Flags: review?(mrbkap)
(Assignee)

Comment 6

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=6a4c729c3e7b
Assignee: nobody → bobbyholley+bmo
(Reporter)

Comment 7

5 years ago
Comment on attachment 783278 [details] [diff] [review]
Use the system error reporter for XBL compilation. v1

Review of attachment 783278 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for fixing this! I have no idea how we could endure this problem :)

::: content/xbl/src/nsXBLDocumentInfo.cpp
@@ +249,5 @@
>    AutoPushJSContext cx(mScriptContext->GetNativeContext());
>  
>    // nsJSEnvironment set the error reporter to NS_ScriptErrorReporter so
>    // we must apparently override that with our own (although it isn't clear 
>    // why - see bug 339647)

Do you know the answer to this? Maybe you should remove this comment.
Comment on attachment 783278 [details] [diff] [review]
Use the system error reporter for XBL compilation. v1

Review of attachment 783278 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/xbl/src/nsXBLDocumentInfo.cpp
@@ +249,5 @@
>    AutoPushJSContext cx(mScriptContext->GetNativeContext());
>  
>    // nsJSEnvironment set the error reporter to NS_ScriptErrorReporter so
>    // we must apparently override that with our own (although it isn't clear 
>    // why - see bug 339647)

Yeah, this comment should go. The bug itself has the answer (NS_ScriptErrorReporter expects the context and global object to be DOM things, which isn't the case for XBL).
Attachment #783278 - Flags: review?(mrbkap) → review+
https://hg.mozilla.org/mozilla-central/rev/57cda2d1472f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.