67647 - [RFE] Make JavaScript Console more useful

Status: VERIFIED FIXED

(Core Graveyard :: Error Console, enhancement)

Description

[Joe Hewitt (gone)]

I got the sudden urge on friday night to make the js console more useful, and so
it became my weekend project.  I set out to add a couple features but wound up
practically rewriting it from scratch. In addition to the new features, I
cleaned up the UI of quite a bit.  The new features are:

* You can copy selected rows to the clipboard 

* The erroneous line of code, if available, is underlined with an arrow pointing
to the column where the error occured (like in 4.x console)

* Clicking on the source file will open it up in the view source window

* A new type of item, a "Message" is supported.  This is just a straight block
of text that has been dumped to the console using nsIConsoleService.  Useful for
debugging in optimized builds when you can't use dump()

* The good ol' javascript entry field from the 4.x console is back

* You can sort the console so that incoming messages appear at either the top or
the bottom

Comment 1

[Joe Hewitt (gone)]

Attachment: patch full of goodness

Comment 2

[Joe Hewitt (gone)]

Attachment: console-caret.gif (put in modern/global and classic/global)

Comment 3

[Joe Hewitt (gone)]

Attachment: console-dash.gif (put in modern/global and classic/global)

Comment 4

[Joe Hewitt (gone)]

Attachment: screen shot of new window, if you don't want to apply the patch

Comment 5

[bht237]

Sorry for the spam, but many thanks for all these great features!!! 

Comment 6

[Jesse Ruderman]

*** Bug 67602 has been marked as a duplicate of this bug. ***

Comment 7

[timeless]

please change [in w3 recommended, but violation of ietf namespace]
language="javascript" => type="text/javascript"

+        <menu value="&toolbarsCmd.label;" accesskey="&toolbarsCmd.accesskey;">
+                         <menupopup> 
eek, tabs :-(

\ No newline at end of file
eek.

And now i'm going to apply and use this wonderful patch!!

Comment 8

[Jesse Ruderman]

Two minor things that worry me from the screenshot:

- white/grey alternation in lists is often used to make rows (or groups of two 
rows) more visible and easier to follow.  The screenshot shows a white "row" 
and a grey "row" as part of the same message.  That might be a little 
misleading.

- I see blocks at the end of the "erroneous lines".  Are those newline 
characters?

Comment 9

[Peter "jag" Annema]

hewitt: did you remove the Index: lines from this patch? It makes applying the
patch very hard. After I added them back, I still couldn't apply cleanly, for
some reason it barfs on consoleBindings.xml. Would you mind attaching an updated
patch? Thanks :-)

Comment 10

[timeless]

Attachment: address my complaints and assign a few accesskeys.

Comment 11

[Joe Hewitt (gone)]

Thanks, everyone, for the excellent feedback.  The forthcoming patch
incorporates all of your suggestions, and tweaks a few more things to perfection.

Comment 12

[Joe Hewitt (gone)]

Attachment: patch, rev2

Comment 13

[[e-mail]]

Surely Console:Clear should not be a type="radio" menuitem?
(not that it is ever checked, so no-one will notice :-)

Comment 14

[[e-mail]]

I'll add these here becuase you might have noticed and/or fixed these already:
1. Clicking Warnings or Errors hides but does not collapse other messsages.
2. No keyboard accelerators (Ctrl+W etc).

Comment 15

[Joe Hewitt (gone)]

The only thing that I don't consider finished about my patch is the way that
type-in javascript in the toolbar is executed.  I'm just using eval, which is
problematic because if there's an error in the code it will show up in the
console as being from chrome://global/content/console.js, which is obviously
wrong.  There should be no source document.  I'll have to figure out a good way
to execute js code "anonymously" without crediting a document, the same way it's
done if you type a url into navigator like "javascript: blah()"

Comment 16

[Robert Ginda]

Chatzilla wraps its eval() call  (for the /eval command) in a try/catch.  You
could try changing the fileName property of the exception and rethrow it (I'm
not positive that'd work), or just print the error to the console.

You probably want to check that the fileName and lineNo properties of the
exception line up with the eval() call, so you don't hide exceptions thrown from
functions called by the eval().

Comment 17

[Joe Hewitt (gone)]

Is it possible to change the fileName property of exceptions in Javascript? I
thought our exception objects were just strings?

Comment 18

[Joe Hewitt (gone)]

A thought I have, which I'm not sure will work, but which I am sure is a hack,
is to put a hidden iframe or browser in the xul and pass it a javascript url. 
Or perhaps there's an easier way to pass a javascript url to some other sort of
url loader.

Comment 19

[Robert Ginda]

Exceptions can be any jsval.  The engine throws Error objects, which look
something like this...

+ message (string) 'missing ; before statement'
+ fileName (string) 'chrome://chatzilla/content/handlers.js'
+ lineNumber (number) 1027
+ name (string) 'SyntaxError'
*

Comment 20

[Joe Hewitt (gone)]

I changed the script evaluation so that it uses an iframe to evaluate the
script, rather than using eval().  This gives a nice clean evaluation with an
independent js context, so that the evaluated script doesn't have access to the
dom of the console window.  

Can I please get some reviewage for the updated patch? I need to check this in
by Thursday because of the localization freeze (because I added several labels).

Comment 21

[Joe Hewitt (gone)]

Attachment: patch, rev3

Comment 22

[Joe Hewitt (gone)]

Thanks to jag for a thorough review of this bug.  I've made a batch of changes
at his request, and received a gracious r=jag.  Here comes the latest patch. 
sr= anyone?

Comment 23

[Joe Hewitt (gone)]

Attachment: patch, rev4

Comment 24

[Peter "jag" Annema]

One final comment:

+            msg += "\n" + strBundle.getString("errFile") + " " +
this.getAttribute("url") +
+                   "\n" + strBundle.getString("errLine") + " " +
this.getAttribute("line");

Instead of that you you should use something like:

errFile=Source file: %S
errLine=Source line: %S

and then in your JS:

+            msg += "\n" + strBundle.getFormattedString("errFile",
[this.getAttribute("url")]) +
+                   "\n" + strBundle.getFormattedString("errLine",
[this.getAttribute("line")]);

etc.

You could (should?) probably use it for errSeparator too (errSeparator=, %S),
though you may want to consult i18n on this.

Comment 25

[timeless]

combining two intl formatted strings is probably a big NoNo.

errLocation=Source file: %S; line: %S
this way, if some language needs RTL vs LTR they can do it.

Comment 26

[Peter "jag" Annema]

+    if (frame.filename)
+      str += frame.filename + ", Line " + frame.lineNumber;

Is getStackTrace debugging code?

+          if (this.hasAttribute("line") && this.hasAttribute("url")) {
+            msg += "\n" + strBundle.getString("errFile") + " " +
this.getAttribute("url") +  
+                   "\n" + strBundle.getString("errLine") + " " +
this.getAttribute("line");
+            if (this.hasAttribute("col")) {
+              msg += strBundle.getString("errSeparator") + " " +
+                     strBundle.getString("errColumn") + " " +
this.getAttribute("col");
+            }

So thinking about this, you have something like:
File: <filename>
Line: <linenr>[, Column: <colnr>]

So have:

errLine=Line: %S
errLineCol=Line: %S, Column: %S

and then make the js select the right one to format with:

  [this.getAttribute("line")]

and

  [this.getAttribute("line"), this.getAttribute("col")]

respectively. And then you can get rid of errSeparator.

Comment 27

[Joe Hewitt (gone)]

Attachment: patch, rev1,000,000

Comment 28

[Peter "jag" Annema]

I've seen revs 6,834; 170,145; and 800,001; but I have no idea what happened to
the rest.

r=jag on rev 1,000,000.

Comment 29

[HJ]

This is great, but one small issue, why didn't you changed year to 2001?

> Copyright (C) 1998-1999 Netscape Communications Corporation. All
> Rights Reserved.

Comment 30

[timeless]

bad:
+  <script language="javascript" 
src="chrome://global/content/strres.js"></script>
much less bad:
+<script type="text/javascript" 
src="chrome://global/content/globalOverlay.js"/>

Comment 31

[Joe Hewitt (gone)]

fixed. woohoo!

Comment 32

[HJ]

If you right click on a link in the console, a new window opens. I link to be
able to copy that link into the urlbar, can that be done?

Comment 33

[[e-mail]]

I can't use the links at all, because mozilla won't recognise the js mime type.
Is that because
a) I need to associate .js files with the right type
b) I need to upgrade from build 2001031008
c) Some other bug needs fixing?

Comment 34

[Boris Zbarsky [:bzbarsky]]

I think it's 'c'

Comment 35

[timeless]

Currently recommended:
<script type="application/x-javascript" 
src="chrome://global/content/globalOverlay.js"/>
Sorry for not commenting about the change.

Comment 36

[Jesse Ruderman]

Neil, I think that's bug 77080, "Show application/x-javascript in browser 
window instead of trying to download."

Comment 37

[Christopher Aillon (sabbatical, not receiving bugmail)]

Verify fixed for quite a while now.  :)

Comment 38

[Brant Gurganus]

[RFE] is deprecated in favor of severity: enhancement.  They have the same meaning.