No line numbers for Syntax Errors

VERIFIED FIXED in Firefox 30

Status

defect
P3
normal
VERIFIED FIXED
6 years ago
10 months ago

People

(Reporter: freddyb, Assigned: recursive)

Tracking

({regression})

Trunk
Firefox 30
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good-first-bug] [mentor=bbenvie@mozilla.com] [lang=js] )

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
I am testing and debugging JS in Scratchpad, but when I do stupid things like syntax errors I don't get a line number. In this case it was something like |"foo":+ bar|, where the colon should have went into the string.

I took me a while to figure that out. I wouldn't have wasted my that much time if I was any good in JS. But it's also a bug in Scratchpad :P


Bugzilla says Bug 864680 might be related, but I'm not sure.
Screenshot: http://i.imgur.com/Q9XIKgB.png
Experienced both in Aurora (recent) and Nightly (a day or two behind)

Comment 1

5 years ago
This is a regression: such line numbers used to be displayed.
Keywords: regression
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk

Updated

5 years ago
Duplicate of this bug: 950926
This is happening because of how Scratchpad.writeAsErrorComment interprets the error's "stack" property [1]. Previously, the check was something like:

> if (error.stack) {
>   stack = error.stack;
> } else {
>   // assemble stack from other info like error.lineNumber
> }

The check was changed to `if (typeof error.stack == "string") {`. The problem with that is SyntaxErrors have the empty string as their stack property, so it uses that instead of building a more useful one.

Fixing this just requires changing the check back, and we should add a test for SyntaxError formatting to [2].

[1] http://mxr.mozilla.org/mozilla-central/source/browser/devtools/scratchpad/scratchpad.js#847
[2] http://mxr.mozilla.org/mozilla-central/source/browser/devtools/scratchpad/test/browser_scratchpad_display_outputs_errors.js
Priority: -- → P3
Whiteboard: [good-first-bug] [mentor=bbenvie@mozilla.com] [lang=js]

Comment 4

5 years ago
With that change I get things like:

/*
Exception: syntax error
@10
*/

It should say something more like "@Scratchpad/1:10".
(Assignee)

Comment 5

5 years ago
Hi I would like to work on this bug. I'm quite new. Can you please guide me?
Flags: needinfo?(bbenvie)
(In reply to Rahid Hasan [:recursive] from comment #5)
> Hi I would like to work on this bug. I'm quite new. Can you please guide me?
Assignee: nobody → rahid.robin
Flags: needinfo?(bbenvie)
(In reply to Alan Thomas from comment #4)
> With that change I get things like:
> 
> /*
> Exception: syntax error
> @10
> */
> 
> It should say something more like "@Scratchpad/1:10".

Oh wow you're right. There's another bug... `typeof error.fileName == "number"`. Instead of string.
Just checking to see if you still want to do this. I'll unassign it if not.
Flags: needinfo?(rahid.robin)
(Assignee)

Comment 9

5 years ago
Yes. I am working on this.
Flags: needinfo?(rahid.robin)
(Assignee)

Comment 10

5 years ago
Attachment #8364439 - Flags: review?(bbenvie)
Comment on attachment 8364439 [details] [diff] [review]
No line numbers for Syntax Errors; r=benvie

Review of attachment 8364439 [details] [diff] [review]:
-----------------------------------------------------------------

We should add a test to confirm this behavior in the future. You should be able to add a syntax error test to [1].

[1] http://mxr.mozilla.org/mozilla-central/source/browser/devtools/scratchpad/test/browser_scratchpad_display_outputs_errors.js

::: browser/devtools/scratchpad/scratchpad.js
@@ +844,5 @@
>          }
>  
>          // Assemble the best possible stack we can given the properties we have.
>          let stack;
> +        if (error.stack) {

We still also need to check for typeof == "string" here.
Attachment #8364439 - Flags: review?(bbenvie)

Comment 12

5 years ago
Posted patch Example tests (obsolete) — Splinter Review
This is an example of some tests that could be useful.
Benvie, I think attachment 8364851 [details] [diff] [review] needs to be reviewed maybe?
Flags: needinfo?(bbenvie)
Status: NEW → ASSIGNED
(In reply to Victor Porof [:vp] from comment #13)
> Benvie, I think attachment 8364851 [details] [diff] [review] needs to be reviewed maybe?

Ah yeah I was looking for recursive to come back, but with the existing reviewed patch and the new one for the tests from Alan, this should be enough.
Flags: needinfo?(bbenvie) → needinfo?(rahid.robin)
(Assignee)

Comment 15

5 years ago
Benvie, I think as I'm quite inexperienced. Better to move with Alan's patch as it seems good :)
Flags: needinfo?(rahid.robin)
No worries, I'll take care of it. Thanks!
Posted patch no_line_number_error.patch (obsolete) — Splinter Review
Incorporate the tests from Alan. https://tbpl.mozilla.org/?tree=Try&rev=3e814af148a3
Attachment #8364439 - Attachment is obsolete: true
Attachment #8364851 - Attachment is obsolete: true
Attachment #8370244 - Flags: review+
Adds the type check on error.stack back in.
Attachment #8370244 - Attachment is obsolete: true
Attachment #8370288 - Flags: review+
 https://hg.mozilla.org/integration/fx-team/rev/f1156ec42c25
Whiteboard: [good-first-bug] [mentor=bbenvie@mozilla.com] [lang=js] → [good-first-bug] [mentor=bbenvie@mozilla.com] [lang=js] [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/f1156ec42c25
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [good-first-bug] [mentor=bbenvie@mozilla.com] [lang=js] [fixed-in-fx-team] → [good-first-bug] [mentor=bbenvie@mozilla.com] [lang=js]
Target Milestone: --- → Firefox 30
/*
 * This is a JavaScript Scratchpad...
 */

4 = 3
/*
Exception: invalid assignment left-hand side
@Scratchpad/1:5
*/
----
Just to confirm this fixed for me in
navigator.userAgent
/*
Mozilla/5.0 (Windows NT 5.1; rv:30.0) Gecko/20100101 Firefox/30.0
*/
navigator.buildID
/*
20140206030201
*/
Status: RESOLVED → VERIFIED

Comment 22

5 years ago
Currently Scratchpad displays either 3-number or 2-number line numbers for syntax errors: 
eg.
> @Scratchpad/3:1:1
> @Scratchpad/3:1

As I've seen, the 1st number (3) is always the times Scratchpad has been launched in the current session, 
i.e 3 means that during the current Firefox session, Scratchpad has been launched 3 times, i.e the current is the 3rd
(the rest numbers are the line number and/or column number where the error was found).
-I don't mean Scratchpad instances: just closing+reopening Scratchpad 3 times-.

Is the first number needed?
It's very confusing.
(In reply to Kostas from comment #22)
> Currently Scratchpad displays either 3-number or 2-number line numbers for
> syntax errors: 
> eg.
> > @Scratchpad/3:1:1
> > @Scratchpad/3:1
> 
> As I've seen, the 1st number (3) is always the times Scratchpad has been
> launched in the current session, 
> i.e 3 means that during the current Firefox session, Scratchpad has been
> launched 3 times, i.e the current is the 3rd
> (the rest numbers are the line number and/or column number where the error
> was found).
> -I don't mean Scratchpad instances: just closing+reopening Scratchpad 3
> times-.
> 
> Is the first number needed?
> It's very confusing.

Please present the test case which goes along with above errors, which is the exact text content of that scratchpad.

The first number identifies the individual Scratchpad.

Scratchpads can be restored after a restart of firefox using the same profile.

Use File->Exit so that firefox will restore all tabs upon next launch.

> > @Scratchpad/3:1:1
tells me you have an error at column 1 of line 1.

> > @Scratchpad/3:1
tells me you have an error at line 1, column number unspecified.

Also, what is your firefox version and platform?

Try pressing Ctrl+J in Scratchpad while an error comment is still selected after evaluation.

It should prefill the jump to line field.

Just press Enter, and there you are, at the exact error location.

----Scratchpad/6----
my bad

/*
Exception: missing ; before statement
@Scratchpad/6:1
*/
--------

----Scratchpad/7----
let it = be

/*
Exception: be is not defined
@Scratchpad/7:1:9
*/
--------
(In reply to adrian.aichner from comment #23)
> (In reply to Kostas from comment #22)
> > Currently Scratchpad displays either 3-number or 2-number line numbers for
> > syntax errors: 
> > eg.
> > > @Scratchpad/3:1
> tells me you have an error at column 1 of line 1.
> 
> > > @Scratchpad/3:1
> tells me you have an error at line 1, column number unspecified.
> 
> Also, what is your firefox version and platform?
> 
> Try pressing Ctrl+J in Scratchpad while an error comment is still selected
> after evaluation.
> 
> It should prefill the jump to line field.
> 
> Just press Enter, and there you are, at the exact error location.

Works for me in firefox 33.1.1 already.

Comment 25

5 years ago
I use Firefox 33.1.1 (in windows 8.1x64)

> Please present the test case which goes along with above errors, which is
> the exact text content of that scratchpad.
There's not a specific test case where I get these two errors.
I just listed that in general the format of line numbers for syntax errors 
can only be either 2-number or 3-number.


> The first number identifies the individual Scratchpad.
That's what I also described in my comment.


> Scratchpads can be restored after a restart of firefox using the same
> profile.
> 
> Use File->Exit so that firefox will restore all tabs upon next launch.
It's great and very useful that Firefox also restores Scratchpads upon next launch.

But, my thinking is this:
the line numbers currently are in either of these two formats:
> @Scratchpad/S:x:y 
> @Scratchpad/S:x
where: 
S identifies the individual Scratchpad
x shows the line number
y shows the column number

i.e in all cases S is visible,
even when y is omitted (if it's unspecified).

But, for me the user, while working with code,
my concern is only the line number (x) and/or column number (y) where the error has occurred.
'S' is of no use to me.
In fact, displaying it all the time (when 'y' is omitted) : 
> @Scratchpad/S:x
confuses the user who might think that the error is:
> @Scratchpad/x:y
instead (in other words, he might think that S=x and x=y)


So, my suggestion is this: S to be omitted
and the line numbers to have the following format:
> @Scratchpad/x:y
> @Scratchpad/x:-  (when column number is omitted)
Have you tried Ctrl+J when an evaluation error is selected (e.g. after Ctrl+R)?

Try.

Comment 27

5 years ago
(In reply to adrian.aichner from comment #26)
> Have you tried Ctrl+J when an evaluation error is selected (e.g. after
> Ctrl+R)?
> 
> Try.
Yes, and it's very useful. Thanks for the tip.

My suggestion still stands though: can't the 'S' be omitted from the displayed error?
Is it necessary for the shortcut Ctrl+J to work?? (but, the "prefilled jump to line field" is in the format x:y or plain x, not S:x:y )
Can't 'S' be converted into an internal variable(not visible to the user)?
(In reply to Kostas from comment #27)
> (In reply to adrian.aichner from comment #26)
> > Have you tried Ctrl+J when an evaluation error is selected (e.g. after
> > Ctrl+R)?
> > 
> > Try.
> Yes, and it's very useful. Thanks for the tip.
> 
> My suggestion still stands though: can't the 'S' be omitted from the
> displayed error?
> Is it necessary for the shortcut Ctrl+J to work?? (but, the "prefilled jump
> to line field" is in the format x:y or plain x, not S:x:y )
> Can't 'S' be converted into an internal variable(not visible to the user)?

Consider this:

Two Scratchpads running code against same firefox tab content and logging facts to the console.

How could I figure out from the log in the console I actually ran Scratchpad/3, then Scratchpad/4, then Scratchpad/3 again?

,--- Scratchpad/3
// Run some code, then state:
console.log("I'm here!")
// Who knows what else I am doing?
`---

,--- Scratchpad/4
// Run some *other code*, possibly having side effects on code in Scratchpad/3, then state:
console.log("I'm here!")
`---

,---Console
"I'm here!" Scratchpad/3:1
"I'm here!" Scratchpad/4:1
"I'm here!" Scratchpad/3:1
`---

Pretty easy to figure out how I used the scratchpads from just looking at the console.

Now drop the /N part and things are not at all clear anymore.

Comment 29

5 years ago
Kostas and Adrian: Your discussion is not relevant to this specific bug report, which is about line numbers not being provided for Scratchpad errors. 

Please file a new bug for your issue, if appropriate, or take your conversation to another venue. Thanks!

Comment 30

5 years ago
Adrian, ok then. I didn't think of the case of two Scratchpads running simultaneously. 
Thanks for your time explaining this.

Updated

10 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.