Last Comment Bug 228205 - Redesign nsIConsoleService and related APIs
: Redesign nsIConsoleService and related APIs
Status: NEW
[firebug-p2]
: helpwanted, topembed+
Product: Toolkit
Classification: Components
Component: Error Console (show other bugs)
: Trunk
: All All
: -- normal with 41 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 435896 (view as bug list)
Depends on: 342259
Blocks: 222989 231376 302211 435025 466533 122213 147015 158744 260046 493414 529086
  Show dependency treegraph
 
Reported: 2003-12-11 13:36 PST by Boris Zbarsky [:bz]
Modified: 2016-03-08 14:06 PST (History)
65 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Boris Zbarsky [:bz] 2003-12-11 13:36:40 PST
The goal here is to solve a bunch of bugs and make the console service easier to
use in the process by making callers not have to deal with intl directly.  The
proposed changes are as follows:

Allow callers to just pass to the console sevice the following information in a
single function call:

     1)  [wstring] Module logging the message
     2)  [wstring] Message ID 
     3)  [array_of(wstring)] Array of message-specific info strings
     4)  [long] Length of said array    
     5)  [wstring] URL of thing triggering the error (eg script)
     6)  [wstring] Actual source text that led to the error
     7)  [unsigned long] Line number
     8)  [unsigned long] Column number
     9)  flags (though maybe we can eliminate this?)
     10) Document object associated with the error (possibly null).

The plan is that the console service will use items 1-4 to construct the actual
error text (item 1 determines the stringbundle to use, item 2 determines the
string in that bundle, then make intl do some work).  This means callers don't
need to depend on intl.

Item 10 will be used in two ways:
  A) Get the document's docshell (if any) and walk up the same-type
     docshell tree till it gets to its root (the content area), then
     will get the url of the document loaded there and associate it with the
     error (this makes reloading and framesets happy while allowing easy
     location of the errors that apply to the "window from which you opened JS
     console").
  B) If item #5 matches the URI of the document in item #10, the document's
     docshell, if it exists, will be QIed to nsIWebPageDescriptor
     and the current descriptor will be associated with the error (make view
     source happy for POST pages).

What the console service will do with all that info is to create an
nsIConsoleMessage (which will look a hell of a lot like what nsIScriptError
looks like right now).  In addition to what nsIScriptError has now, the console
message, at creation time, will remember the timestamp.  This nsIConsoleMessage
will be dispatched to console service listeners, who all currently handle
nsIScriptErrors with missing info anyway, so should be easily changeable to
handle this.

nsIScriptError will probably be eliminated.

In addition to the above, there will be a method on the console service that
takes a nsIConsoleMessage param (created somehow somewhere).  This can be used
by people who somehow want/need to subclass nsIConsoleMessage.

There will no longer be a logStringError message.  Callers will log an error as
above with 0 or null for most of the fields (but non-null module and message id).

A "clear()" method will be added to the console service that clears its current
set of stored messages.  Then we can remove the evil JS console hack.

The only really problematic thing here is item 10 in the argument list. 
Currently the console service is in XPCOM, and it can't depend on things like
nsIDOMDocument (for scriptability), nsIDocument, nsIDocShell, etc.  The proposal
is to leave the IDL where it is (so XPConnect can still use it), make that param
an nsISupports documented to be a document object, and move the implementation
out into embedding/components where it can depend on all this garbage.

brendan, what do you think?
Comment 1 Christian :Biesinger (don't email me, ping me on IRC) 2003-12-11 14:54:32 PST
>     5)  [wstring] URL of thing triggering the error (eg script)

maybe nsIURI? a |wstring| looses the charset information. oh... this is xpcom...
Comment 2 Alex Vincent [:WeirdAl] 2004-02-29 13:59:38 PST
bz: Would it be reasonable to add the error's stack property to that list?  See 
bug 125612.
Comment 3 Boris Zbarsky [:bz] 2004-02-29 14:03:01 PST
Maybe.... This is about errors in general, not just JS errors.  So most of them
will not have a stack property.

For that matter, is the stack on Error() objects a JS-only thing?  Or can it be
reflected into C++ somehow?  (I suspect the former.)
Comment 4 Alex Vincent [:WeirdAl] 2004-02-29 14:06:55 PST
:) See bug 229824.
Comment 5 Boris Zbarsky [:bz] 2004-02-29 14:08:20 PST
That's still talking about JS-only objects.

Once there is a reasonable API that lets me pass these stack traces around in
C++, we can add them to nsIConsoleService.
Comment 6 Benjamin Smedberg [:bsmedberg] 2004-03-09 11:38:33 PST
Is there a reason why this interface has to live in XPCOM? It doesn't look like
XPCOM itself uses the interface for anything (though I can imagine the component
manager/loader probably should). If not, can't we push the entire interface+impl
into embedding/components, which is tier_9 and solves all the dependencies.

We're gonna need something like this so that the ffox extension manager can
report meaningful errors in a non-synchronous way... unless somebody else wants
to step up to the plate, I'm gonna hack together a not-quite-perfect solution
very soon.

I also question the need for 6-7-8: can't we just make those part of 2?
Comment 7 Boris Zbarsky [:bz] 2004-03-09 11:42:41 PST
Can xpconnect usefully depend on embedding/components?
Comment 8 Boris Zbarsky [:bz] 2004-03-09 11:45:00 PST
What do you mean by making 6-7-8 part of 2?  2 is an identifier for the type of
error (data that can be used to look up the error message to report or
whatever).  6-7-8 are information about the circumstances in which the error
occured (for display by the JS console, for example).  I see no relationship...
Comment 9 Brendan Eich [:brendan] 2004-03-09 11:47:04 PST
> Is there a reason why this interface has to live in XPCOM?

So XPConnect can depend on it, as bz said in comment 0.  No, I don't think we
want XPConnect to depend on later tiers.  We've had bugs already where we've had
to ifdef it bogusly.

Still, XPCOM is only one place we could put it that satisfies all dependencies.
 We could also put it in XPConnect, I suppose, but it ought to be independent of
JS some day (soon, if Python integration for XUL proceeds well).  Doug should
comment on putting an abstracted nsIConsoleService.idl, but no impl, in XPCOM.

There may be another equally low tier we can put the .idl file under -- thoughts?

/be
Comment 10 cls 2004-03-09 20:20:32 PST
(In reply to comment #9)
> There may be another equally low tier we can put the .idl file under -- thoughts?

'fraid not unless you're going to create a new one.  js & xpcom live in tier_2
and xpconnect is the first thing built after that...in tier_9.
Comment 11 Brendan Eich [:brendan] 2004-03-09 22:51:22 PST
Ok, how about xpcom/io/nsIConsoleService.idl?  xpcom/io already hosts interfaces
not implemented in XPCOM, but used by XPCOM.  It's legit to have a more
primitive library expose a "callback" or set of callbacks.  Doug, what do you say?

/be
Comment 12 Doug Turner (:dougt) 2004-03-09 23:15:06 PST
xpcom/io is fine for the nsIConsoleService IDL.  As you mentioned in comment #9,
the implementation should live else where.
Comment 13 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-12 22:55:55 PDT
Maybe nsIStackFrame would be useful here? Oh, and for #10, in stead of a
document object, how about just an nsISupports argument just as a general error
context, or what not? All errors may not always be related to a document...
Comment 14 Boris Zbarsky [:bz] 2004-07-12 23:15:35 PDT
We could make it a general context, but in the end we want to be able to get the
document URI and the docshell from the context thing, whatever it is...

nsIStackFrame would work to encapsulate some of the args if we want that sort of
thing, yes.
Comment 15 Christian :Biesinger (don't email me, ping me on IRC) 2004-07-13 07:15:20 PDT
(In reply to comment #14)
> We could make it a general context, but in the end we want to be able to get the
> document URI and the docshell from the context thing, whatever it is...

then make it an interface requestor and document what people should be prepared
to return?
Comment 16 Boris Zbarsky [:bz] 2004-07-13 13:04:14 PDT
Getting an interface requestor off a document is tough and a lot of work for
some of the callers.  I'd rather do nsISupports and say what it should be
prepared to QI to.
Comment 17 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-13 14:31:06 PDT
Yeah, say that it's either a nsIDOMNode or an nsIInterfaceRequestor, or
something along those lines...
Comment 18 Christian :Biesinger (don't email me, ping me on IRC) 2005-01-23 17:22:07 PST
Can we also add an nsIStackFrame argument to nsIConsoleService? That way, it
would be possible to show a stack trace for the error leading to the console
message.
Comment 19 Joe Hewitt 2006-01-28 17:46:43 PST
Fixing this bug would be a huge help to the FireBug extension (https://addons.mozilla.org/extensions/moreinfo.php?id=1843&application=firefox).

The goal of the extension is to show the errors that derive from each window/tab in a separate console that is in a panel at the bottom of the browser window.  Since nsIScriptError does not report the document that the error came from, FireBug's error reporting is not always accurate.  The best I can do is append errors to the console that is currently visible, assuming it is most probable that errors will come from the document you are looking at.

Comment 20 ark 2006-01-28 18:08:50 PST
I would also vote to fix this bug, I started using the extension mentioned in comment 19, it's very useful, unfortunately this bug causes it to report wrong error count for windows out of focus.
Comment 21 Boris Zbarsky [:bz] 2006-01-29 21:04:31 PST
I'm not going to be working on this any time in the near future.  It'd be nice to have for 1.9, but I doubt I'll manage it.  If someone wants to take this and implement per the plan, that would be great.
Comment 22 Philip Chee 2008-08-01 21:30:42 PDT
SeaMonkey::Error Console->Toolkit::Error Console

On trunk we use the toolkit error console.
Comment 23 John J. Barton 2008-08-02 22:59:03 PDT
To re-iterate and amplify Joe's comment 19, a central challenge for dev tools  is the mismatch between how developers think and how the consoleService responds. Almost all development work is related to a window, except perhaps the lowest platform layers that start before the first window. So almost all errors start in some window, get thrown into nsIConsoleService without a window ref, then land in a place where the developer needs to figure out which window the error came from. Firebug works hard to reconstruct the information that the error source already has. In addition to getting it wrong sometimes, this also a significant performance hit since lots of sites are pumping lots of errors via ajax on to pages Firebug users aren't even debugging.
Comment 24 John J. Barton 2008-08-02 23:00:42 PDT
*** Bug 435896 has been marked as a duplicate of this bug. ***
Comment 25 Boris Zbarsky [:bz] 2008-08-03 20:44:23 PDT
Comment 22: thanks for taking this out of my bug list.  Was there a reason you did that?

Comment 23: That's what this bug is largely about, yes.
Comment 26 David Dahl :ddahl 2010-05-20 15:32:21 PDT
*** Bug 567165 has been marked as a duplicate of this bug. ***
Comment 27 Steve Roussey (:sroussey) 2010-10-14 20:34:41 PDT
Cross referencing bug reports:
http://code.google.com/p/fbug/issues/detail?id=1572
Comment 28 Sebastian Zartner 2011-07-25 02:35:47 PDT
Another cross referencing:
http://code.google.com/p/fbug/issues/detail?id=1213

Note You need to log in before you can comment on or make changes to this bug.