Tom S [:evilpie] Reporter
	Description • 11 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.

	Tom S [:evilpie] Reporter
	Comment 1 • 11 years ago

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.

	Boris Zbarsky [:bzbarsky]
	Comment 2 • 11 years ago

Bobby, thoughts?  I thought we had an XBL error reporter for this stuff...

	Bobby Holley (:bholley) Assignee
	Comment 3 • 11 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?

	Tom S [:evilpie] Reporter
	Comment 4 • 11 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.

	Bobby Holley (:bholley) Assignee
	Comment 5 • 11 years ago

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.

	Bobby Holley (:bholley) Assignee
	Comment 6 • 11 years ago

https://tbpl.mozilla.org/?tree=Try&rev=6a4c729c3e7b

	Tom S [:evilpie] Reporter
	Comment 7 • 11 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.

	Blake Kaplan (:mrbkap) (inactive)
	Comment 8 • 11 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]:
-----------------------------------------------------------------

::: 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).

	Bobby Holley (:bholley) Assignee
	Comment 9 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/57cda2d1472f

	Ed Morley [:emorley]
	Comment 10 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/57cda2d1472f