Summary: | Web Inspector: Implement position-based sourcemapping for stylesheets | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexander Pavlov (apavlov) <apavlov> | ||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Alexander Pavlov (apavlov) <apavlov> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | apavlov, buildbot, dglazkov, keishi, loislo, pfeldman, pmuellr, rniwa, vsevik, web-inspector-bugs, webkit.review.bot, yurys | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 105285 | ||||||||||||
Attachments: |
|
Description
Alexander Pavlov (apavlov)
2013-02-07 03:44:59 PST
Created attachment 187051 [details]
Patch
Comment on attachment 187051 [details] Patch Attachment 187051 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16426170 New failing tests: http/tests/inspector/stylesheet-source-mapping.html http/tests/inspector/compiler-script-mapping.html Comment on attachment 187051 [details] Patch Attachment 187051 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16432194 New failing tests: http/tests/inspector/stylesheet-source-mapping.html http/tests/inspector/compiler-script-mapping.html Comment on attachment 187051 [details] Patch Attachment 187051 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/16427217 Comment on attachment 187051 [details] Patch Attachment 187051 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16415378 New failing tests: http/tests/inspector/stylesheet-source-mapping.html http/tests/inspector/compiler-script-mapping.html Comment on attachment 187051 [details] Patch Attachment 187051 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16431326 New failing tests: http/tests/inspector/stylesheet-source-mapping.html http/tests/inspector/compiler-script-mapping.html Created attachment 187281 [details]
For EWS
Comment on attachment 187281 [details] For EWS Attachment 187281 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16427593 New failing tests: http/tests/inspector/stylesheet-source-mapping.html Created attachment 187291 [details]
Patch
Comment on attachment 187291 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187291&action=review > Source/WebCore/inspector/front-end/CSSStyleModel.js:543 > var sourceMapping = this._sourceMappings[rawLocation.url]; We should always fallback to default mapping. if (sourceMapping) { var uiLocation = sourceMapping.rawLocationToUILocation(rawLocation); if (uiLocation) return uiLocation; } > Source/WebCore/inspector/front-end/SASSSourceMapping.js:170 > + if (sourceMap && (forceRebind || !this._sourceMapByStyleSheetURL[cssURL])) { Why would we go to loadSourceMapForStyleSheet() when (!forceRebind && this._sourceMapByStyleSheetURL[cssURL]) holds in the first place? if (!forceRebind && this._sourceMapByStyleSheetURL[cssURL]) return; var sourceMap = this.loadSourceMapForStyleSheet(match[1], cssURL, forceRebind); if (!sourceMap) return; this._sourceMapByStyleSheetURL[cssURL] = sourceMap; this._bindUISourceCode(cssURL, sourceMap); > Source/WebCore/inspector/front-end/SASSSourceMapping.js:210 > + var payload = WebInspector.SourceMap.loadPayload(completeSourceMapURL); We can now rename loadPayload to SourceMap.load() and move this 4 lines there, like function loadSourceMap(sourceMapURL, compiledURL) { var payload = ... // load payload for sourceMapURL if (!payload) return null; var baseURL = sourceMapURL.startsWith("data:") ? compiledURL : completeSourceMapURL; return new WebInspector.PositionBasedSourceMap(baseURL, payload); } We should also remove RangeBasedSourceMap since we don't need it anymore and merge PositionBasedSourceMap and SourceMap. > Source/WebCore/inspector/front-end/SASSSourceMapping.js:254 > + if (!entry) if (!entry || entry.length === 2) return null; > LayoutTests/http/tests/inspector/stylesheet-source-mapping.html:21 > + function checkMapping(compiledLineNumber, compiledColumnNumber, sourceURL, sourceLineNumber, sourceColumnNumber, mapping) Unused? > LayoutTests/http/tests/inspector/stylesheet-source-mapping.html:81 > + InspectorTest.checkUILocation(scssUISourceCode, 0, 0, rawLocationToUILocation(0, 0)); Should be InspectorTest.checkUILocation(cssUISourceCode, 0, 0, rawLocationToUILocation(0, 0)), this is why it does not fail although the code is incorrect. Created attachment 187528 [details]
Patch
Comment on attachment 187528 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187528&action=review > Source/WebCore/inspector/front-end/SASSSourceMapping.js:218 > + this._sourceMapByURL[completeSourceMapURL] = sourceMap; This line should probably be before previous return (in case we already had a sourceMap but forceReload flag was set) Committed r142445: <http://trac.webkit.org/changeset/142445> |