Closed Bug 977463 Opened 11 years ago Closed 7 years ago

SourceMaps generated from commonjs-everywhere not working

Categories

(DevTools :: Framework, defect, P3)

30 Branch
x86
macOS
defect

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
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
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)
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)
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: WORKSFORME → ---
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.
See also this issue in the mozilla/source-map library: https://github.com/mozilla/source-map/issues/62
Hi, Can you reproduce this issue with the new version of Firefox?
Flags: needinfo?(jiyinyiyong)
Meanwhile, James Long can you take a look at this? Thank you.
Component: Untriaged → JavaScript Engine
Flags: needinfo?(jlong)
Product: Firefox → Core
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)
"Web compatibility" (or, source-maps-in-the-wild compatibility) requires doing URI resolving.
Flags: needinfo?(nfitzgerald)
Flags: needinfo?(jiyinyiyong)
Blocks: source-maps
Component: JavaScript Engine → Developer Tools: Framework
Priority: -- → P3
Product: Core → Firefox
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.
This is going to be fixed upstream, then landed in M-C via one of the bundle updates.
I believe this is fixed now.
Assignee: nobody → ttromey
Status: REOPENED → RESOLVED
Closed: 11 years ago7 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.