Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] Reporter
	Description • 12 years ago

When there is a source map for the script being debugged, the debugger should use it to display the original source and allow break points to be set referencing the original source.

	Panos Astithas (he/him) [:past] (please ni?)
	Comment 1 • 12 years ago

One way to do it would be to keep an additional map in the thread actor next to _scripts, and retrieve the original file in TA__addScript. After that the client would receive an additional 'unminified' property (or whatever) in the 'scripts' response or the 'newScript' notification.

This assumes a post-bug 755661 world, where the script text is retrieved through the protocol and not by the client independently, as we currently do. This world is not that far off, actually.

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] Reporter
	Comment 2 • 12 years ago

(In reply to Panos Astithas [:past] from comment #1)
> One way to do it would be to keep an additional map in the thread actor next
> to _scripts, and retrieve the original file in TA__addScript. After that the
> client would receive an additional 'unminified' property (or whatever) in
> the 'scripts' response or the 'newScript' notification.

This makes sense to me.

Because multiple original sources can create a single generated source (for example, module imports concat'd together), I imagine this property would be an object of the form { <original source url> : <original source contents> } or something similar. I think we'd also need to add a _sourceMaps property to ThreadActor as well, which would be of the form { <script> : <SourceMapConsumer> } where <script> would be some way of hashing a Debugger.Script (urls wouldn't work if we want to support eval'd scripts).

> This assumes a post-bug 755661 world, where the script text is retrieved
> through the protocol and not by the client independently, as we currently
> do. This world is not that far off, actually.

Are you saying that this bug should depend on bug 755661?

Obviously I'm not as familiar with the debugger as you, but I'm not sure that should be the case here. The original sources are never actually executed by spidermonkey so the problem of the debugger and the debuggee getting different versions of the same script doesn't really apply here. Even after that bug is fixed, I still imagine the client would be the one fetching the original sources.

Am I overlooking something?

	Panos Astithas (he/him) [:past] (please ni?)
	Comment 3 • 12 years ago

(In reply to Nick Fitzgerald :fitzgen from comment #2)
> (In reply to Panos Astithas [:past] from comment #1)
> > One way to do it would be to keep an additional map in the thread actor next
> > to _scripts, and retrieve the original file in TA__addScript. After that the
> > client would receive an additional 'unminified' property (or whatever) in
> > the 'scripts' response or the 'newScript' notification.
> 
> This makes sense to me.
> 
> Because multiple original sources can create a single generated source (for
> example, module imports concat'd together), I imagine this property would be
> an object of the form { <original source url> : <original source contents> }
> or something similar. I think we'd also need to add a _sourceMaps property
> to ThreadActor as well, which would be of the form { <script> :
> <SourceMapConsumer> } where <script> would be some way of hashing a
> Debugger.Script (urls wouldn't work if we want to support eval'd scripts).

Makes sense.

> > This assumes a post-bug 755661 world, where the script text is retrieved
> > through the protocol and not by the client independently, as we currently
> > do. This world is not that far off, actually.
> 
> Are you saying that this bug should depend on bug 755661?

If we put source map handling in the server (the script actors) we need to have protocol support for returning script source to the client in order for the editor to display it. This is the main part of the work that I intend to do in bug 755661: move the current script-fetching code in the server (until bug 637572 is done) and extend the remote debugging protocol to allow the client to retrieve it. If we end up with source map handling in the client, then the right place for source map handling would not be the script actors, but the thread client in dbg-client.jsm, or even debugger-controller.js if we don't want to attach it to the debugging protocol.

> Obviously I'm not as familiar with the debugger as you, but I'm not sure
> that should be the case here. The original sources are never actually
> executed by spidermonkey so the problem of the debugger and the debuggee
> getting different versions of the same script doesn't really apply here.
> Even after that bug is fixed, I still imagine the client would be the one
> fetching the original sources.
> 
> Am I overlooking something?

The problem is not that SpiderMonkey may execute the original sources, but that the debugger server is running in the same process as the remote browser. In bug 755661 Mark mentions both accessibility issues (script may not be retrieved by the desktop browser due to missing authentication cookies or even network ACLs) and user agent sniffing issues. I can see both still being valid for source maps: the former for obvious reasons and the latter for files with generic names. For example, if a foo-min.js has a foo.js unminified file and a foo-map.js source map in the minified file, then user agent sniffing would cause the desktop browser to retrieve a different unminified file, that would point to a different source map. Obviously source maps specified with HTTP headers wouldn't face this issue.

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] Reporter
	Comment 4 • 12 years ago

Kevin,

Panos and I had a conversation on IRC discussing using source maps with scripts where there are multiple versions sent based on the user agent of the requester. For example, if you request foo-min.js on mobile, it might be different than foo-min.js on desktop.

I have edited the IRC logs a bit to clean them up.

This was our original plan:

  fitzgen: as long as the debugger server (on mobile) provides the correct source map url, the debugger client (on desktop) can fetch the source map, and from there the original sources, and everything will be correct
  fitzgen: because the source map can be linked to a script through the source or through an http header
  fitzgen: so that's why we need to extend the protocol
  fitzgen: just add something to the protocol that maps a script to a source map url
  fitzgen: and let the debugger client handle everything involving source maps from there

But then Panos thought of one more scenario:

  past: fitzgen: so, I think there is still a case where this plan won't work:
  fitzgen: yeah?
  past: site serves foo-min.js, which points to foo.map, which points to foo.js
  past: but their server sends a different version for *each* of those files depending on the user agent
  past: including the source map
  past: is that something people do?
  fitzgen: past: I don't think so
  fitzgen: I'd imagine they only serve foo-min.js like that
  past: I wonder if they put all files in /desktop and /mobile paths
  fitzgen: yeah, thats the only use case I can imagine that could lead to that situation
  past: but yeah, if that's not something we're worried about, I'm fine with this plan
  fitzgen: past: how much can we be responsible for? personally, I think that if we make sure we get the url for the source map for the file we are actually debugging, we have done as much as should be expected
  past: that's a reasonable argument
  past: fitzgen: I'd say let's get Kevin's thoughts on this
  past: he may want to do some user research or something

And so, we ask your advice on the matter :)

	Rob Campbell [:rc] (:robcee)
	Comment 5 • 12 years ago

oh yes.

	Kevin Dangoor
	Comment 6 • 12 years ago

(In reply to Nick Fitzgerald :fitzgen from comment #4)
>   past: site serves foo-min.js, which points to foo.map, which points to
> foo.js
>   past: but their server sends a different version for *each* of those files
> depending on the user agent
>   past: including the source map
>   past: is that something people do?
>   fitzgen: past: I don't think so
>   fitzgen: I'd imagine they only serve foo-min.js like that
>   past: I wonder if they put all files in /desktop and /mobile paths
>   fitzgen: yeah, thats the only use case I can imagine that could lead to
> that situation
>   past: but yeah, if that's not something we're worried about, I'm fine with
> this plan
>   fitzgen: past: how much can we be responsible for? personally, I think
> that if we make sure we get the url for the source map for the file we are
> actually debugging, we have done as much as should be expected
>   past: that's a reasonable argument
>   past: fitzgen: I'd say let's get Kevin's thoughts on this
>   past: he may want to do some user research or something
> 
> And so, we ask your advice on the matter :)

I think this scenario seems less likely, and I also think that people will ultimately change their build output if there is new tools support that makes their lives better.

So, if we support the same kinds of build output that Chrome does, then I think it's fine for us to get the feature out the door and see if anyone has a good reason for this kind of build setup.

	Brian Slesinsky
	Comment 7 • 11 years ago

Hi, what's the current status? With Firefox 23 it looks like the debugger shows the source code, but breakpoints don't work right.

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] Reporter
	Comment 8 • 11 years ago

Brian, initial support was introduced in Firefox 23, but it was semi-incomplete. We have been improving support in every release since then. Can you reproduce your problems on Nightly? If so, please file a bug w/ a test case and steps to reproduce so we can fix it.

	Brian Slesinsky
	Comment 9 • 11 years ago

I don't have full repro steps yet and for GWT this would be a bit involved, but here are some observations from playing around with GWT's Super Dev Mode and nightly 26.0a1 (2013-08-13):
 
- The Java source shows up fine when I turn sourcemaps on. (The order of the files on the left is unclear; they aren't obviously grouped by directory and they aren't always in alphabetical order either. Fortunately search works.)
- If I set a breakpoint in the Java source, it appears in the listing the left, but the breakpoint has no apparent effect.
- If I turn sourcemaps off, there's no corresponding breakpoint listed in the JavaScript file.
- If I turn sourcemaps back on and remove the Java breakpoint, it doesn't work. The breakpoint doesn't disappear on the left side, and if I reload the page, the breakpoint reappears.
- If I turn off breakpoints, set a breakpoint in the JavaScript, then turn sourcemaps back on, it stops at the correct place in the Java code.

	Brian Slesinsky
	Comment 10 • 11 years ago

Also, saw this stack trace:

Error: 'delete' request packet has no destination.: DC_request@resource://gre/modules/devtools/dbg-client.jsm:581
@resource://gre/modules/devtools/dbg-client.jsm:328
Breakpoints.prototype.removeBreakpoint@chrome://browser/content/devtools/debugger-controller.js:1441
Breakpoints.prototype.destroy@chrome://browser/content/devtools/debugger-controller.js:1247
DebuggerView._destroyEditor@chrome://browser/content/devtools/debugger-view.js:195
DebuggerView.destroy@chrome://browser/content/devtools/debugger-view.js:82
DebuggerController.shutdownDebugger@chrome://browser/content/devtools/debugger-controller.js:122
DebuggerController._onTabDetached@chrome://browser/content/devtools/debugger-controller.js:239
EventEmitter_emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/shared/event-emitter.js:107
TabTarget.prototype.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/target.js:407
TBOX_destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:824

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] Reporter
	Comment 11 • 11 years ago

This meta bug doesn't need an assignee.

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] Reporter
	Comment 12 • 10 years ago

Eddy, bug 833744 is about //# sourceURL which is distinct from //# sourceMappingURL and has nothing to do with source maps. It's exposed to JS via the Debugger.Source.prototype.displayURL property.

	Eddy Bruel [:ejpbruel]
	Comment 13 • 10 years ago

(In reply to Nick Fitzgerald [:fitzgen] from comment #12)
> Eddy, bug 833744 is about //# sourceURL which is distinct from //#
> sourceMappingURL and has nothing to do with source maps. It's exposed to JS
> via the Debugger.Source.prototype.displayURL property.

Thanks for pointing that out Nick. I had assumed it had to do with source maps because the name is obviously very similar, and you mentioned the source mapping mailing list in the comments. I'll do a bit more investigation next time ;-)

	Eddy Bruel [:ejpbruel]
	Comment 14 • 10 years ago

Glad I'm not the only pedant here when it comes to bug summaries Nick! :-P

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 15 • 9 years ago

Marking this as a feature to help with tracking, QA, and general awareness of active/upcoming work.