[source maps] _parseMap in SourceMap.js does not strictly follow specs
Reported by
antonin....@gmail.com,
May 13 2016
|
|||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.94 Safari/537.36 Steps to reproduce the problem: Currently mappings parser reuses previous name if nameIndex is not present in current segment. But this is wrong behaviour. According to specs[1] name should be assigned to segment only if nameIndex is present. Current behaviour: https://github.com/binaryage/dirac/blob/15c21bc73f606ab8791219279d4dbbecb900718b/resources/unpacked/devtools/front_end/sdk/SourceMap.js#L460-L463 Proposed fix: https://github.com/binaryage/dirac/commit/5d12448fafb8720a00aeee90f5d21113db059f16 Background story: I have encountered this issue when consuming ClojureScript source maps. ClojureScript being a LISP can generate code via macros during compilation. Because the source was expanded, there is no mapping from generated javascript variables to original source code variables. This _parseMap bug caused assignment of immediate previous name to these "unmapped" variables which should have no name assigned. I have also consulted Mozilla's implementation of their source-map[2] library and they strictly follow the specs[3]. [1] https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit?pli=1#heading=h.qz3o9nc69um5 [2] https://github.com/mozilla/source-map [3] https://github.com/mozilla/source-map/blob/aa0398ced67beebea34f0d36f766505656c344f6/lib/source-map-consumer.js#L505-L509 What is the expected behavior? What went wrong? Resulted in incorrect name mappings read from ClojureScript source maps. Did this work before? N/A Chrome version: 52.0.2734.0 Channel: canary OS Version: OS X 10.11.5 Flash Version: Shockwave Flash 21.0 r0
,
May 16 2016
Andrey, please take a look.
,
Aug 5 2016
Would you like to upload a patch? The link on how to do this: https://docs.google.com/document/d/1WNF-KqRSzPLUUfZqQG5AFeU_Ll8TfWYcJasa_XGf7ro/view#heading=h.cpwtdeg8y01b
,
Aug 23 2016
,
Aug 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/51f1de9d9288bc3603f38d30341cc61e960bd44d commit 51f1de9d9288bc3603f38d30341cc61e960bd44d Author: antonin.hildebrand <antonin.hildebrand@gmail.com> Date: Tue Aug 30 04:07:38 2016 [devtools] _parseMap in SourceMap.js does not strictly follow specs Also add tests for issue 611738 BUG= 611738 Review-Url: https://codereview.chromium.org/2222713004 Cr-Commit-Position: refs/heads/master@{#415074} [modify] https://crrev.com/51f1de9d9288bc3603f38d30341cc61e960bd44d/AUTHORS [modify] https://crrev.com/51f1de9d9288bc3603f38d30341cc61e960bd44d/third_party/WebKit/LayoutTests/http/tests/inspector/compiler-script-mapping-expected.txt [modify] https://crrev.com/51f1de9d9288bc3603f38d30341cc61e960bd44d/third_party/WebKit/LayoutTests/http/tests/inspector/compiler-script-mapping.html [modify] https://crrev.com/51f1de9d9288bc3603f38d30341cc61e960bd44d/third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js
,
Aug 31 2016
,
Aug 31 2016
Thanks very much for hunting this down and contributing this fix!
,
Aug 31 2016
Thanks. Now I will be likely to attach patches to reported issues in future :-) |
|||
►
Sign in to add a comment |
|||
Comment 1 by antonin....@gmail.com
, May 13 2016