Closed Bug 898811 Opened 11 years ago Closed 11 years ago

JS method parsing failure is swallowed

Categories

(Core :: XBL, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: evilpie, Assigned: bholley)

References

Details

Attachments

(2 files)

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.
Blocks: 895548
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)
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)
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.
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: nobody → bobbyholley+bmo
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+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: