Closed Bug 1202827 Opened 9 years ago Closed 6 years ago

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

Categories

(DevTools :: Console, defect)

43 Branch
defect
Not set
normal

Tracking

(firefox43 affected)

RESOLVED WONTFIX
Tracking Status
firefox43 --- affected

People

(Reporter: fayolle-florent, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

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
Attached patch 1202827.patch (obsolete) — Splinter Review
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)
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)
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
Attached patch 1202827.patchSplinter Review
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
Closed: 6 years ago
Resolution: --- → WONTFIX
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: