1346936 - Support "SourceMap" header (along with" X-SourceMap")

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Florian Scholz (former MDN Content Lead)]

It looks like bug 765993 introduced the X-SourceMap header.

WebKit also supports just "SourceMap", https://bugs.webkit.org/show_bug.cgi?id=114888

Unsure what Chrome does, but maybe we want to support the unprefixed version, too.

Comment 1

[Tom Tromey :tromey]

I think we do want to support it.
X-SourceMap is deprecated:
https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/

Comment 2

[Tom Tromey :tromey]

Attachment: Bug 1346936 - support SourceMap header in addition to X-SourceMap;

According to the source map spec, the X-SourceMap header is deprecated
in favor of SourceMap.  This adds support for the newer name.  The test
case is a copy of the X-SourceMap test with the obvious change.

Review commit: https://reviewboard.mozilla.org/r/139326/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139326/

Comment 3

[:ehsan akhgari]

Comment on attachment 8867810 [details]
Bug 1346936 - support SourceMap header in addition to X-SourceMap;

https://reviewboard.mozilla.org/r/139326/#review142712

Code-wise, looks great!

Can you please send an intent to ship for this to dev-platform as per https://wiki.mozilla.org/WebAPI/ExposureGuidelines?  Thanks!

::: dom/script/ScriptLoader.cpp:2736
(Diff revision 1)
>  
>      nsAutoCString sourceMapURL;
> +    rv = httpChannel->GetResponseHeader(NS_LITERAL_CSTRING("SourceMap"), sourceMapURL);
> +    if (NS_FAILED(rv)) {
> -    rv = httpChannel->GetResponseHeader(NS_LITERAL_CSTRING("X-SourceMap"), sourceMapURL);
> +      rv = httpChannel->GetResponseHeader(NS_LITERAL_CSTRING("X-SourceMap"), sourceMapURL);
> +    }

Do other UAs have this exact behavior (using SourceMap header and ignoring X-SourceMap if that exists, and otherwise falling back to X-SourceMap)?  I'm r+ing based on this assumption.

I'd appreciate if you can please clarify the specifcs in the intent to ship thread.

Comment 4

[Tom Tromey :tromey]

> Do other UAs have this exact behavior (using SourceMap header and ignoring X-SourceMap if that exists, and otherwise falling back to X-SourceMap)?  I'm r+ing based on this assumption.

I found this in chromium:

https://chromium.googlesource.com/chromium/src/third_party/+/master/WebKit/Source/bindings/core/v8/ScriptSourceCode.cpp#57

Still cloning webkit to dig in there, but given the path name, I would assume it does the same.

Comment 5

[:ehsan akhgari]

(In reply to Tom Tromey :tromey from comment #4)
> > Do other UAs have this exact behavior (using SourceMap header and ignoring X-SourceMap if that exists, and otherwise falling back to X-SourceMap)?  I'm r+ing based on this assumption.
> 
> I found this in chromium:
> 
> https://chromium.googlesource.com/chromium/src/third_party/+/master/WebKit/
> Source/bindings/core/v8/ScriptSourceCode.cpp#57

Thanks for checking!

> Still cloning webkit to dig in there, but given the path name, I would
> assume it does the same.

If you mean the /WebKit/ in the path name, that doesn't mean much of anything any more after Blink forked WebKit.  Not much more than "ns" in our class names meaning NetScape.  :-)

Comment 6

[Tom Tromey :tromey]

> If you mean the /WebKit/ in the path name, that doesn't mean much of anything any more after Blink forked WebKit

Thanks.  I didn't know how much overlap there might still be.

Webkit does the same, see:

https://github.com/WebKit/webkit/blob/master/Source/WebCore/inspector/PageDebuggerAgent.cpp#L82-L88

I'll send the intent email soon.

Comment 7

[:ehsan akhgari]

Awesome, thanks for checking!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ea7440b56ab
support SourceMap header in addition to X-SourceMap; r=Ehsan

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5ea7440b56ab

Comment 10

[Florian Scholz (former MDN Content Lead)]

Developer release notes
https://developer.mozilla.org/en-US/Firefox/Releases/55#Developer_Tools

Reference page
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/SourceMap

Compat data
https://github.com/mdn/browser-compat-data/pull/229