support //# sourceURL annotations in eval'd scripts in Console

RESOLVED FIXED in Firefox 58

Status

P3
normal
RESOLVED FIXED
6 years ago
2 months ago

People

(Reporter: rcampbell, Assigned: tromey)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 58

Firefox Tracking Flags

(firefox58 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
We should support eval'd scripts marked with //@ sourceURL in the Console.

e.g., 

function someFunc() {
  debugger;
}
//@ derp.js

would appear as derp.js in the console's source field if added via the input line, window.eval() or some other mechanism.
s/@/#/ as per discussion in dev-js-sourcemap.

See also, http://www.softwareishard.com/blog/firebug/firebug-tip-label-dynamic-scripts-with-sourceurl-directive/ which has a nice little regexp for us.
Summary: support //@ sourceURL annotations in eval'd scripts in Console → support //# sourceURL annotations in eval'd scripts in Console
(Reporter)

Updated

5 years ago
Priority: -- → P3
Also, the original example is bad, you want:

function foo() {
  debugger;
}
//# sourceURL=http://example.com/derp.js
(Assignee)

Updated

a year ago
Blocks: 1339970
(Assignee)

Comment 3

11 months ago
Something weird about this bug is that if I follow the STR in comment #2, then it does fail.
But if I wrap the code in `eval("..")` -- it works.
(Assignee)

Updated

11 months ago
Assignee: nobody → ttromey
Comment hidden (mozreview-request)

Comment 5

11 months ago
mozreview-review
Comment on attachment 8909432 [details]
Bug 833747 - recognize "debugger eval" as eval source;

https://reviewboard.mozilla.org/r/180938/#review186150

looks good!
Attachment #8909432 - Flags: review?(jlaster) → review+
(Assignee)

Comment 6

11 months ago
So that failure from the old debugger looks like:

eval('debugger; //# sourceURL=foo.js')

... where the test expects this to show up as http://example.com/foo.js

This seems marginal to me, so I might just tweak the test and move on.

However, I'm of two minds, because I think some other, bigger changes are needed
in this area; and so maybe it's better to just batch them all up into one series.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1192882#c5 for the gory details
(Assignee)

Comment 7

11 months ago
Specifically what is marginal about it is that it is a debugger eval script.
I think it's probably fine for this to show up as "(no domain)" rather
than being under the document's URL.

Jason, what do you think?
Flags: needinfo?(jlaster)
(Assignee)

Comment 8

10 months ago
I think it's best to just implement it.
Flags: needinfo?(jlaster)
Comment hidden (mozreview-request)

Comment 10

10 months ago
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f13bdb40d572
recognize "debugger eval" as eval source; r=jlast
https://hg.mozilla.org/mozilla-central/rev/f13bdb40d572
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58

Updated

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