567165 - Collect and report "missing" CSS Parsing Errors to the Web Console

Status: RESOLVED DUPLICATE of bug 606498

(DevTools :: General, defect, P1)

Description

[David Dahl :ddahl]

It would be very helpful to get the original scope or window object as a property of each nsIConsoleMessage.

Perhaps there is a better way to know the scope or window, but as it stands I will have to do some pretty hackish bookkeeping to know the originating browser window of each js error, css warning, etc...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Note that nsIConsoleMessages often don't come from JS, and that JS itself often doesn't have access to the information you want in the obvious way...

Also note that in the e10s world what you want doesn't make much sense.

I do think we need something like this (see bug 228205), but it needs a good bit of under-the-hood rework to happen.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I pretty carefully didn't mark this duplicate, because the scope here is a lot narrower than bug 228205...

Comment 4

[David Dahl :ddahl]

ok, i'll re-open. If I could make it even narrower: perhaps just getting the new Window ID (bug 534149) as a property of each 'content javascript' or CSS parser nsIConsoleMessage would be an easier/quicker patch?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Yes.  Even that would be reasonably involved, I think (depending on how you define "content javascript").  But at least there you only really have two error-reporting callsites, for the most part.

Comment 6

[John J. Barton]

How about just walking the JS stack (up and over any native frames), back to the oldest frame, then out to the most enclosing scope: that's your answer? If your goal is to assign an error to the top_level call scope.

Comment 7

[Boris Zbarsky [:bzbarsky]]

What js stack?  See comment 1.  A large fraction of the messages we're talking about here have nothing whatsoever to do with JS.

Comment 8

[David Dahl :ddahl]

After talking to sayre yesterday, I am going to try to add an onerror handler to each window as it is created, keeping any existing onerror to fire after mine. Sayre also mentioned that there might be a platform fix (in mind) where an event is fired each time an error is thrown and perhaps for each css parsing error - this way we can avoid listening to the nsIConsoleService altogether.

Comment 9

[Boris Zbarsky [:bzbarsky]]

onerror won't catch the CSS things, nor lots of other stuff that goes to the JS console.  It'll only catch js errors (plus some load errors, depending on how you set it up).

How is a DOM event easier than using the console service?

Comment 10

[Benjamin Smedberg]

I suggested to ddahl that we send DOM events with CSS errors (probably not onerror, but some chrome-specific event). That is, assuming that we actually know, from the CSS code which has the error, which window(s) it can be associated with.

That way we don't have the leak problems associated with console error objects entraining windows.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Console error objects could have window ids; that's the whole point of window ids.  DOM events are just as hard to do on the CSS end as getting the window id, of course...  Except it would take more code, to make sure that the event got dispatched at a safe time, etc.

Comment 12

[Dietrich Ayala (:dietrich)]

Blocking+. Script errors are a P1 feature of the console and we can't ship the feature without them.

Comment 13

[David Dahl :ddahl]

Dietrich and I were talking about a work-around for this:

as each script and link uri is enumerated by the httpActivityObserver, we can inventory each uri and tie it to each WebConsole (HeadsUpDisplay).

When we encounter an error whose source contentWindow is not apparent to us, we can look in the "link-script" array or object for matching WebConsoles and log it accordingly.

We will have to keep an eye on timing and DOMContentLoaded events, as I assume errors can emanate before all DOM content is loaded, correct?

This method will result in some false positives, but that is ok for a temporary patch that some future platform work will fix.

Comment 14

[John J. Barton]

(In reply to comment #13)
> Dietrich and I were talking about a work-around for this:
> 
> as each script and link uri is enumerated by the httpActivityObserver, we can
> inventory each uri and tie it to each WebConsole (HeadsUpDisplay).
> 
> When we encounter an error whose source contentWindow is not apparent to us, we
> can look in the "link-script" array or object for matching WebConsoles and log
> it accordingly.

This is essential what Firebug does in errors.js using its context and context.sourceFileMap arrays.

> 
> We will have to keep an eye on timing and DOMContentLoaded events, as I assume
> errors can emanate before all DOM content is loaded, correct?

No, only errors related to loading come before DOMContentLoaded. The others come at other times ;-)

> 
> This method will result in some false positives, but that is ok for a temporary
> patch that some future platform work will fix.

Anytime a URL appears twice in the browser this scheme fails, eg two tabs on the same site. It also fails for chrome and resource URLs which accompany some kinds of system errors. Your two bad choices are suppress the error or report it against all windows. Either way your users will complain.

Comment 15

[David Dahl :ddahl]

(In reply to comment #14)

> Anytime a URL appears twice in the browser this scheme fails, eg two tabs on
> the same site. It also fails for chrome and resource URLs which accompany some
> kinds of system errors. Your two bad choices are suppress the error or report
> it against all windows. Either way your users will complain.

we will report it to both windows, and that will be a little bit bad. I expect this (interim) patch to be short lived.

Comment 16

[David Dahl :ddahl]

saving a bt from a crash while working on this patch, nothing to see here:

(gdb) bt
#0  0xb7865422 in __kernel_vsyscall ()
#1  0xb45efb86 in poll () from /lib/tls/i686/cmov/libc.so.6
#2  0xb492f4eb in g_poll () from /lib/libglib-2.0.so.0
#3  0xb49220ac in ?? () from /lib/libglib-2.0.so.0
#4  0xb49224b8 in g_main_context_iteration () from /lib/libglib-2.0.so.0
#5  0xb66cc449 in nsAppShell::ProcessNextNativeEvent (this=0xb0bf2420, mayWait=1) at /home/ddahl/code/moz/mozilla-central/mozilla-central/widget/src/gtk2/nsAppShell.cpp:144
#6  0xb66f3533 in nsBaseAppShell::DoProcessNextNativeEvent (this=0xb0bf2420, mayWait=1)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:161
#7  0xb66f396c in nsBaseAppShell::OnProcessNextEvent (this=0xb0bf2420, thr=0xb3cef790, mayWait=1, recursionDepth=0)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:317
#8  0xb6a32e71 in nsThread::ProcessNextEvent (this=0xb3cef790, mayWait=1, result=0xbf9ea5bc) at /home/ddahl/code/moz/mozilla-central/mozilla-central/xpcom/threads/nsThread.cpp:517
#9  0xb69beac9 in NS_ProcessNextEvent_P (thread=0xb3cef790, mayWait=1) at nsThreadUtils.cpp:250
#10 0xb684d785 in mozilla::ipc::MessagePump::Run (this=0xb3cdba00, aDelegate=0xb3c277c0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/ipc/glue/MessagePump.cpp:134
#11 0xb6a976bf in MessageLoop::RunInternal (this=0xb3c277c0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/ipc/chromium/src/base/message_loop.cc:219
#12 0xb6a9763f in MessageLoop::RunHandler (this=0xb3c277c0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/ipc/chromium/src/base/message_loop.cc:202
#13 0xb6a975e3 in MessageLoop::Run (this=0xb3c277c0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/ipc/chromium/src/base/message_loop.cc:176
#14 0xb66f35cc in nsBaseAppShell::Run (this=0xb0bf2420) at /home/ddahl/code/moz/mozilla-central/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:180
#15 0xb6443859 in nsAppStartup::Run (this=0xb0aa3b20) at /home/ddahl/code/moz/mozilla-central/mozilla-central/toolkit/components/startup/src/nsAppStartup.cpp:191
#16 0xb543e935 in XRE_main (argc=4, argv=0xbf9ead64, aAppData=0xb3c10380) at /home/ddahl/code/moz/mozilla-central/mozilla-central/toolkit/xre/nsAppRunner.cpp:3665
#17 0x08049a57 in main (argc=4, argv=0xbf9ead64) at /home/ddahl/code/moz/mozilla-central/mozilla-central/browser/app/nsBrowserApp.cpp:158

Comment 17

[David Dahl :ddahl]

Attachment: 0.0 Initital debugging patch

Posting this to move to another machine as my OBJ DIR is totally borked.

Comment 18

[David Dahl :ddahl]

Attachment: v 0.0 Saving work

Comment 19

[David Dahl :ddahl]

I wanted to mention that I wrote a test with 2 included scripts and a linked CSS file. I had the two included scripts each call a method that does not exist. Each was caught by our onerror handler and reported to the WebConsole output correctly.


The included CSS had some borked selectors and the errors thrown were not handled correctly. So, we may only have issues with included CSS now.

I will write a more comprehensive test to drive these questions.

Comment 20

[David Dahl :ddahl]

Attachment: v 0.1 a good start.

Note to self: try to get the loadGroup of the current file to match it with a WebConsole output node

Comment 21

[David Dahl :ddahl]

Attachment: WORKING PATCH - careful it is raw and ugly. (saving work)

Just making people excited about code!

Comment 22

[John J. Barton]

It seems like this bug has morphed into "When reporting errors, warnings, etc... to the HUD, report the originating window or context", ie it won't help anyone using nsIConsoleService

Comment 24

[David Dahl :ddahl]

Changing this to devtools for now. Will open a new bug for platform work.