605241 - JavaScript strict warnings from external scripts do not show in the Web Console

Status: VERIFIED FIXED

(DevTools :: General, defect)

Description

[Mihai Sucan [:msucan]]

Currently we use the window.onerror event handler to work around the problem with the nsIConsoleService observer which fails to catch errors from external scripts.

However, the window.onerror event handler fails to catch warnings. We need to fix this.

Comment 1

[David Dahl :ddahl]

(In reply to comment #0)
> However, the window.onerror event handler fails to catch warnings. We need to
> fix this.

This is not going to be pretty, probably will be a hack like bug 567165

Comment 2

[Boris Zbarsky [:bzbarsky]]

Why doesn't the console service catch errors from external scripts?  Because it has no way to map them back to the relevant page?

I haven't seen any bugs on core code asking for that information to be available... the code that sends the error to the console service sure knows what page is involved, and could presumably expose that information on the error (though at this point the needed API changes will be a lot more painful than they would have been earlier in the beta cycle).  Is there a reason to not do that?  Then you won't need the window.onerror thing at all _and_ will get a fix for this bug, no?

Comment 3

[Boris Zbarsky [:bzbarsky]]

And in general, could you guys _please_ talk to someone when you have a problem like this instead of looking for workarounds?  That's exactly the problem we've had with Firebug in the past, but at least they have the limited excuse of wanting to develop against older gecko revisions, etc...

Comment 4

[Rob Campbell [:rc] (:robcee)]

Well Boris, I appreciate you offering to help out.

We probably didn't realize we *needed* that information until now or didn't know how to form the bug request. That said, I would be happy to file a bug on this now.

Also, in the future, if you notice something that we could make use of, by all means, please file it and CC us on it, ping us in IRC or send us an email.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Will do.  Please do file a bug?  It should be pretty simple to add an inner window id to all the JS errors/warnings being reported to the console service...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Er, might have to be an outer window id.  For your purposes it should be equivalent.

Comment 7

[Rob Campbell [:rc] (:robcee)]

(In reply to comment #5)
> Will do.  Please do file a bug?  It should be pretty simple to add an inner
> window id to all the JS errors/warnings being reported to the console
> service...

filed bug 605492. Please modify / adjust as you see fit. Not sure if some of the wording in there makes sense.

Comment 8

[Mihai Sucan [:msucan]]

Thanks Boris for offering to help us out with this kind of issues!


(In reply to comment #4)
> Well Boris, I appreciate you offering to help out.
> 
> We probably didn't realize we *needed* that information until now or didn't
> know how to form the bug request. That said, I would be happy to file a bug on
> this now.
> 
> Also, in the future, if you notice something that we could make use of, by all
> means, please file it and CC us on it, ping us in IRC or send us an email.

We have a bug report about this idea: please see bug 603706. The specific details of how to do it differ - either a reference to the originating window object, or just the window ID, it's really up to the implementer how he decides we should track the originating window.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

So does this bug now cover making use of the functionality added in bug 605492, and removing the onerror handlers? I think that makes it a blocker...

Comment 10

[:Gavin Sharp [email: [e-mail]]]

I guess bug 597136 has a partial patch for that. We should do the work in one of these or the other, and mark the other dependent :)

Comment 11

[Mihai Sucan [:msucan]]

(In reply to comment #10)
> I guess bug 597136 has a partial patch for that. We should do the work in one
> of these or the other, and mark the other dependent :)

Given Patrick will tackle the new nsIScriptError2 API in bug 597136, I'd expect this bug to be fixed with that patch.

Comment 12

[Patrick Walton (:pcwalton)]

Assigning to self. The forthcoming patches in bug 597136 should fix this.

Comment 13

[Robert Sayre]

fixed?

Comment 14

[Mihai Sucan [:msucan]]

Tested this a little and it seems to work fine.

Comment 15

[Rob Campbell [:rc] (:robcee)]

fixed with landing of bug 597136.

thanks for confirming, mihai.