Closed
Bug 977463
Opened 11 years ago
Closed 7 years ago
SourceMaps generated from commonjs-everywhere not working
Categories
(DevTools :: Framework, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jiyinyiyong, Assigned: tromey)
References
(Blocks 1 open bug)
Details
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/34.0.1847.3 Safari/537.36
Steps to reproduce:
I was trying to debug my code with the sourcemaps generated with commonjs-everywhere
https://github.com/Cirru/cirru-parser
Actual results:
But Chrome and firefox tries to fetch the wrong mapping files.
As talked in the issue, the problem was located at the leading slash of sources property:
https://github.com/michaelficarra/commonjs-everywhere/issues/97
Probably you need to read that isssue for details.
Expected results:
In this example
```js
{
"version": 3,
"sources": ["/coffee/live.coffee", "/coffee/parser.coffee"],
"sourceRoot": "..",
```
`/coffee/live.coffee` should always behave like `coffee/live.coffee` according to the protocol of sourcemaps 3
Comment 1•11 years ago
|
||
Having '..' as the source root works fine for me. The leading / is an absolute path, so it is resolved as such. Don't think there is a bug in Firefox here.
Status: UNCONFIRMED → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
Comment 2•11 years ago
|
||
I disagree with this resolution. Please see [my comment][0] from [michaelficarra/commonjs-everywhere#97][1], reproduced in part here:
From the source map specification,
> An optional source root, useful for relocating source
> files on a server or removing repeated values in the
> “sources” entry. This value is prepended to the
> individual entries in the “source” field.
and
> If the sources are not absolute URLs after prepending
> of the “sourceRoot”, the sources are resolved relative
> to the SourceMap (like resolving script src in a html
> document).
[0] https://github.com/michaelficarra/commonjs-everywhere/issues/97#issuecomment-35626341
[1] https://github.com/michaelficarra/commonjs-everywhere/issues/97
Flags: needinfo?(nfitzgerald)
Comment 3•11 years ago
|
||
Ah ok, yeah this is slightly more complicated; I misread before.
The spec does say to prepend, but we've always been doing path joining / url resolution because it is what is generally expected and seen in the wild. I thought I started a thread on the source map list to see if we couldn't change the spec but I can't find it so maybe I didn't.
Flags: needinfo?(nfitzgerald)
Updated•11 years ago
|
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: WORKSFORME → ---
Comment 4•11 years ago
|
||
At the very least, the spec needs to be clarified with respect to joining of paths such as "prepending". Does that mean simple string concatenation or joining by the path separator? I've always assumed the former, which was part of the reason this bug arose.
Comment 5•11 years ago
|
||
See also this issue in the mozilla/source-map library: https://github.com/mozilla/source-map/issues/62
Comment 6•9 years ago
|
||
Hi,
Can you reproduce this issue with the new version of Firefox?
Flags: needinfo?(jiyinyiyong)
Comment 7•9 years ago
|
||
Meanwhile, James Long can you take a look at this?
Thank you.
Component: Untriaged → JavaScript Engine
Flags: needinfo?(jlong)
Product: Firefox → Core
Comment 8•9 years ago
|
||
I've tried to change how we use `sourceRoot` before but it broke some other stuff. I'm not sure I'm the best person to actively work on this. Nick, is this still an issue? I can help talk through what we might be able to change.
Flags: needinfo?(jlong) → needinfo?(nfitzgerald)
Comment 9•9 years ago
|
||
"Web compatibility" (or, source-maps-in-the-wild compatibility) requires doing URI resolving.
Flags: needinfo?(nfitzgerald)
Updated•9 years ago
|
Flags: needinfo?(jiyinyiyong)
Assignee | ||
Updated•8 years ago
|
Blocks: source-maps
Component: JavaScript Engine → Developer Tools: Framework
Priority: -- → P3
Product: Core → Firefox
Assignee | ||
Comment 10•8 years ago
|
||
From reading the source, Chrome seems to follow what the spec says with
https://src.chromium.org/viewvc/blink/trunk/Source/devtools/front_end/sdk/SourceMap.js#l290
That is:
var sourceRoot = sourceMap.sourceRoot || '';
if (sourceRoot && !sourceRoot.endsWith('/'))
sourceRoot += '/';
for (var i = 0; i < sourceMap.sources.length; ++i) {
var href = sourceRoot + sourceMap.sources[i];
var url = Common.ParsedURL.completeURL(this._sourceMappingURL, href) || href;
It took me a while but I managed to set up a test case locally where chrome fetches the
original sources but firefox does not.
Given that: (1) this is what the spec says, (2) firefox's source-map story has generally been
behind the times, and (3) chrome implemented the spec, I think our best choice now is to
change our implementation.
Assignee | ||
Comment 11•7 years ago
|
||
This is going to be fixed upstream, then landed in M-C via one of the bundle updates.
Assignee | ||
Comment 12•7 years ago
|
||
I believe this is fixed now.
Assignee: nobody → ttromey
Status: REOPENED → RESOLVED
Closed: 11 years ago → 7 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•