From f3c05d85a530312994f4cc68b3bce56ef66fa016 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Thu, 15 Jan 2015 21:39:54 +0000 Subject: [PATCH] Fixes #414 - source map position fallback. Apparently there is a problem with building source maps from input source maps when the latter one does not have correct (?) selector positions. It happens to postcss which in case of such a source: ```css selector-1, selector-2, selector-3 { color:red; } ``` generates selector source position for the first selector only. To fix this issue clean-css tries to find out correct selector position going to lines above which may have a selector position. --- History.md | 7 +++- lib/selectors/source-map-stringifier.js | 18 ++++----- lib/utils/input-source-map-tracker.js | 9 +++-- test/source-map-test.js | 50 +++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 13 deletions(-) diff --git a/History.md b/History.md index 1568cee8..6c83a5fc 100644 --- a/History.md +++ b/History.md @@ -1,4 +1,4 @@ -[3.1.0 / 2015-xx-xx](https://github.com/jakubpawlowicz/clean-css/compare/v3.0.4...v3.1.0) +[3.1.0 / 2015-xx-xx](https://github.com/jakubpawlowicz/clean-css/compare/v3.0.5...v3.1.0) ================== * Adds 0deg to 0 minification where possible. @@ -8,6 +8,11 @@ * Fixed issue [#357](https://github.com/GoalSmashers/clean-css/issues/357) - non-standard but valid URLs. * Fixed issue [#416](https://github.com/GoalSmashers/clean-css/issues/416) - accepts hash as `minify` argument. +[3.0.5 / 2015-xx-xx](https://github.com/jakubpawlowicz/clean-css/compare/v3.0.4...v3.0.5) +================== + +* Fixed issue [#414](https://github.com/GoalSmashers/clean-css/issues/414) - source maps position fallback. + [3.0.4 / 2015-01-11](https://github.com/jakubpawlowicz/clean-css/compare/v3.0.3...v3.0.4) ================== diff --git a/lib/selectors/source-map-stringifier.js b/lib/selectors/source-map-stringifier.js index 16ca2d1a..bf9a8b2a 100644 --- a/lib/selectors/source-map-stringifier.js +++ b/lib/selectors/source-map-stringifier.js @@ -37,7 +37,7 @@ Rebuilder.prototype.relativePathResolver = function (sourcePath, sourceRelativeT return path.relative(this.rebaseTo, sourcePath); }; -Rebuilder.prototype.rebuildValue = function (list, separator) { +Rebuilder.prototype.rebuildValue = function (list, separator, isSelector) { var escaped = 0; for (var i = 0, l = list.length; i < l; i++) { @@ -50,19 +50,19 @@ Rebuilder.prototype.rebuildValue = function (list, separator) { if (i === l - 1 && escaped > 0) this.output.splice(this.output.length - escaped - 1, 1); } else { - this.store(el); + this.store(el, isSelector ? i : 0); this.store(i < l - 1 ? separator : ''); escaped = 0; } } }; -Rebuilder.prototype.store = function (token) { +Rebuilder.prototype.store = function (token, allowNFallbacks) { var value = typeof token == 'string' ? token : token.value.indexOf('_') > -1 ? this.restore(token.value) : token.value; - this.track(value, token.metadata); + this.track(value, token.metadata, allowNFallbacks); this.output.push(value); }; @@ -92,7 +92,7 @@ Rebuilder.prototype.rebuildList = function (tokens, isFlatBlock) { this.store('}'); } } else { - this.rebuildValue(token.value, ','); + this.rebuildValue(token.value, ',', true); this.store('{'); this.rebuildValue(token.body, ';'); this.store('}'); @@ -102,18 +102,18 @@ Rebuilder.prototype.rebuildList = function (tokens, isFlatBlock) { } }; -Rebuilder.prototype.track = function (value, metadata) { +Rebuilder.prototype.track = function (value, metadata, allowNFallbacks) { if (metadata) - this.trackMetadata(metadata, value); + this.trackMetadata(metadata, value, allowNFallbacks); var parts = value.split('\n'); this.line += parts.length - 1; this.column = parts.length > 1 ? 0 : (this.column + parts.pop().length); }; -Rebuilder.prototype.trackMetadata = function (metadata, value) { +Rebuilder.prototype.trackMetadata = function (metadata, value, allowNFallbacks) { var original = this.inputMapTracker.isTracking(metadata) ? - this.inputMapTracker.originalPositionFor(metadata, value) : + this.inputMapTracker.originalPositionFor(metadata, value, allowNFallbacks) : {}; this.outputMap.addMapping({ diff --git a/lib/utils/input-source-map-tracker.js b/lib/utils/input-source-map-tracker.js index ff346f6d..4a83c4b9 100644 --- a/lib/utils/input-source-map-tracker.js +++ b/lib/utils/input-source-map-tracker.js @@ -107,7 +107,7 @@ function fetchMapFile(self, mapSource, context, done) { .setTimeout(self.timeout); } -function originalPositionIn(trackedSource, sourceInfo, token) { +function originalPositionIn(trackedSource, sourceInfo, token, allowNFallbacks) { // FIXME: we should rather track original positions in tokenizer // here it is a bit too late do do it reliably hance the hack var originalPosition; @@ -125,6 +125,9 @@ function originalPositionIn(trackedSource, sourceInfo, token) { break; } + if (originalPosition.line === null && sourceInfo.line > 1 && allowNFallbacks > 0) + return originalPositionIn(trackedSource, { line: sourceInfo.line - 1, column: sourceInfo.column }, token, allowNFallbacks - 1); + return originalPosition; } @@ -138,8 +141,8 @@ InputSourceMapStore.prototype.isTracking = function (sourceInfo) { return !!this.maps[sourceInfo.source]; }; -InputSourceMapStore.prototype.originalPositionFor = function (sourceInfo, token) { - return originalPositionIn(this.maps[sourceInfo.source], sourceInfo, token); +InputSourceMapStore.prototype.originalPositionFor = function (sourceInfo, token, allowNFallbacks) { + return originalPositionIn(this.maps[sourceInfo.source], sourceInfo, token, allowNFallbacks); }; module.exports = InputSourceMapStore; diff --git a/test/source-map-test.js b/test/source-map-test.js index 34d4d5fe..3c90f0fe 100644 --- a/test/source-map-test.js +++ b/test/source-map-test.js @@ -529,6 +529,56 @@ vows.describe('source-map') }; assert.deepEqual(minified.sourceMap._mappings._array[1], mapping); } + }, + 'input source map with missing mutliselector input': { + 'topic': new CleanCSS({ sourceMap: '{"version":3,"sources":["source.css"],"names":[],"mappings":"AAAA;;;;IAII,YAAW;EACd"}' }).minify('a,\na:hover,\na:visited\n{\n color: red;\n}'), + 'should have 4 mappings': function (minified) { + assert.lengthOf(minified.sourceMap._mappings._array, 4); + }, + 'should have "a" mapping': function (minified) { + var mapping = { + generatedLine: 1, + generatedColumn: 0, + originalLine: 1, + originalColumn: 0, + source: 'source.css', + name: null + }; + assert.deepEqual(minified.sourceMap._mappings._array[0], mapping); + }, + 'should have "a:hover" mapping': function (minified) { + var mapping = { + generatedLine: 1, + generatedColumn: 2, + originalLine: 1, + originalColumn: 0, + source: 'source.css', + name: null + }; + assert.deepEqual(minified.sourceMap._mappings._array[1], mapping); + }, + 'should have "a:visited" mapping': function (minified) { + var mapping = { + generatedLine: 1, + generatedColumn: 10, + originalLine: 1, + originalColumn: 0, + source: 'source.css', + name: null + }; + assert.deepEqual(minified.sourceMap._mappings._array[2], mapping); + }, + 'should have "color:red" mapping': function (minified) { + var mapping = { + generatedLine: 1, + generatedColumn: 20, + originalLine: 5, + originalColumn: 4, + source: 'source.css', + name: null + }; + assert.deepEqual(minified.sourceMap._mappings._array[3], mapping); + } } }) .addBatch({ -- 2.34.1