The location of a server-side log doesn't point to the source

RESOLVED WONTFIX

Status

()

Firefox
Developer Tools: Console
RESOLVED WONTFIX
3 years ago
5 months ago

People

(Reporter: Florent Fayolle, Unassigned)

Tracking

43 Branch
Points:
---

Firefox Tracking Flags

(firefox43 affected)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
STR:
1. Download this project: https://github.com/fflorent/node-chromelogger-test
2. Run it locally (node server.js)
3. Open http://localhost:7357/
4. Open the DevTools
5. Reload the page
6. Click on the location for this log: "Hello from Node.js v0.10.38 end"
   => Get a "File Not Found"

That's because of the line number appearing in the "view-source:" URL.

Florent
(Reporter)

Comment 1

3 years ago
Created attachment 8658345 [details] [diff] [review]
1202827.patch

And here is the patch. 

As explained in IRC, it still has a bug: the view-source is displayed with a selection starting from the provided line to the end of the file (instead of to the end of the line).

Florent
Assignee: nobody → fayolle-florent
Status: NEW → ASSIGNED
Attachment #8658345 - Flags: review?(jryans)
(Reporter)

Updated

3 years ago
Depends on: 1168872
Comment on attachment 8658345 [details] [diff] [review]
1202827.patch

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

::: toolkit/devtools/webconsole/server-logger.js
@@ +322,5 @@
>      return parsedMessage;
>    },
>  
>    parseBacktrace: function(backtrace) {
> +    let reLocation = /(\s*:\s*\d+)+$/;

Please add a few examples of the formats we're attempting to match as a comment above.

Also, it seems like you would want to *not* include whitespace in your group, right?  Otherwise it will be part of the line / column numbers.
Attachment #8658345 - Flags: review?(jryans)
(Reporter)

Updated

3 years ago
Summary: The location of a server-side log doesn't points to the source → The location of a server-side log doesn't point to the source
(Reporter)

Comment 3

3 years ago
Created attachment 8660465 [details] [diff] [review]
1202827.patch

Thanks! Here is an update.

Florent
Attachment #8658345 - Attachment is obsolete: true
Attachment #8660465 - Flags: review?(jryans)
Comment on attachment 8660465 [details] [diff] [review]
1202827.patch

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

Maybe it's best to add a real test to ensure these cases keep working?

::: toolkit/devtools/webconsole/server-logger.js
@@ +330,5 @@
> +   *
> +   * @param {string} backtrace The backtrace of a log.
> +   * @example
> +   *  <code>backtrace="file:///tmp/foo : 3"</code>
> +   *  returns!

Nit: the ! seems unnecessary

@@ +334,5 @@
> +   *  returns!
> +   *  <code>{ url: "file:///tmp/foo", line: 3, column: undefined }</code>
> +   * @example
> +   *  <code>backtrace="file:///tmp/foo:3:42"</code>
> +   *  returns!

Nit: the ! seems unnecessary
Attachment #8660465 - Flags: review?(jryans) → review+
It looks like the patch for this is lying around for a long time while there wasn't much left to do.
Florent, still interested in finishing this?

Sebastian
Flags: needinfo?(fayolle-florent)
Talked to Florent in person and he said he doesn't have time to finish this, unfortunately. Therefore I'm unassigning him so so that somebody else can step up to finish this patch.

Sebastian
Assignee: fayolle-florent → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(fayolle-florent)
Server logs are removed from the new console frontend. This should be handled by ChromeLogger-like extensions.
Status: NEW → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.