From bb1c294b0aa1a554c7d95a246b6cb19b785a921c Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Sun, 12 Apr 2015 17:16:55 +0100 Subject: [PATCH] Improves `spaceAfterClosingBrace` compatibility handling. It should be handled in stringifier and so it's now. As it turns out the previous way was not giving optimal output when building source maps either. --- lib/clean.js | 2 +- lib/stringifier/helpers.js | 8 ++++---- lib/stringifier/simple.js | 1 + lib/stringifier/source-maps.js | 1 + lib/text/urls-processor.js | 5 ++--- test/integration-test.js | 2 +- test/source-map-test.js | 6 +++--- test/text/urls-processor-test.js | 20 +++----------------- 8 files changed, 16 insertions(+), 29 deletions(-) diff --git a/lib/clean.js b/lib/clean.js index 4a51c227..d5fe4bd2 100644 --- a/lib/clean.js +++ b/lib/clean.js @@ -161,7 +161,7 @@ function minify(context, data) { var commentsProcessor = new CommentsProcessor(context, options.keepSpecialComments, options.keepBreaks, options.sourceMap); var expressionsProcessor = new ExpressionsProcessor(options.sourceMap); var freeTextProcessor = new FreeTextProcessor(options.sourceMap); - var urlsProcessor = new UrlsProcessor(context, options.sourceMap, !options.compatibility.properties.spaceAfterClosingBrace); + var urlsProcessor = new UrlsProcessor(context, options.sourceMap); var urlRebase = new UrlRebase(context); var selectorsOptimizer = new SelectorsOptimizer(options, context); diff --git a/lib/stringifier/helpers.js b/lib/stringifier/helpers.js index d25fedef..b8b3dc81 100644 --- a/lib/stringifier/helpers.js +++ b/lib/stringifier/helpers.js @@ -10,7 +10,7 @@ function hasMoreProperties(tokens, index) { } function afterClosingBrace(token, valueIndex) { - return token[valueIndex][0][token[valueIndex][0].length - 1] == ')'; + return token[valueIndex][0][token[valueIndex][0].length - 1] == ')' || token[valueIndex][0].indexOf('__ESCAPED_URL_CLEAN_CSS') === 0; } function afterComma(token, valueIndex) { @@ -33,8 +33,8 @@ function inFilter(token) { return token[0][0] == 'filter' || token[0][0] == '-ms-filter'; } -function inSpecialContext(token, valueIndex) { - return afterClosingBrace(token, valueIndex) || +function inSpecialContext(token, valueIndex, context) { + return !context.spaceAfterClosingBrace && afterClosingBrace(token, valueIndex) || beforeSlash(token, valueIndex) || afterSlash(token, valueIndex) || beforeComma(token, valueIndex) || @@ -82,7 +82,7 @@ function value(tokens, position, isLast, context) { if (j == m - 1 && isImportant) store('!important', context); - if (j < m - 1 && (inFilter(token) || !inSpecialContext(token, j))) { + if (j < m - 1 && (inFilter(token) || !inSpecialContext(token, j, context))) { store(' ', context); } else if (j == m - 1 && !isLast && hasMoreProperties(tokens, position + 1)) { store(';', context); diff --git a/lib/stringifier/simple.js b/lib/stringifier/simple.js index 80a32dbc..0673996d 100644 --- a/lib/stringifier/simple.js +++ b/lib/stringifier/simple.js @@ -8,6 +8,7 @@ function stringify(tokens, options, restoreCallback) { var context = { keepBreaks: options.keepBreaks, output: [], + spaceAfterClosingBrace: options.compatibility.properties.spaceAfterClosingBrace, store: store }; diff --git a/lib/stringifier/source-maps.js b/lib/stringifier/source-maps.js index 72766645..7d0f0430 100644 --- a/lib/stringifier/source-maps.js +++ b/lib/stringifier/source-maps.js @@ -63,6 +63,7 @@ function stringify(tokens, options, restoreCallback, inputMapTracker) { outputMap: new SourceMapGenerator(), restore: restoreCallback, sourceMapInlineSources: options.sourceMapInlineSources, + spaceAfterClosingBrace: options.compatibility.properties.spaceAfterClosingBrace, store: store }; diff --git a/lib/text/urls-processor.js b/lib/text/urls-processor.js index 841e8c07..eb924c55 100644 --- a/lib/text/urls-processor.js +++ b/lib/text/urls-processor.js @@ -3,11 +3,10 @@ var UrlScanner = require('../utils/url-scanner'); var lineBreak = require('os').EOL; -function UrlsProcessor(context, saveWaypoints, removeTrailingSpace) { +function UrlsProcessor(context, saveWaypoints) { this.urls = new EscapeStore('URL'); this.context = context; this.saveWaypoints = saveWaypoints; - this.removeTrailingSpace = removeTrailingSpace; } // Strip urls by replacing them by a special @@ -61,7 +60,7 @@ UrlsProcessor.prototype.restore = function (data) { var url = normalize(this.urls.restore(nextMatch.match)); tempData.push(url); - cursor = nextMatch.end + (this.removeTrailingSpace && data[nextMatch.end] == ' ' ? 1 : 0); + cursor = nextMatch.end; } return tempData.length > 0 ? diff --git a/test/integration-test.js b/test/integration-test.js index 0bd16b3f..9e6b679d 100644 --- a/test/integration-test.js +++ b/test/integration-test.js @@ -2096,7 +2096,7 @@ title']{display:block}", 'advanced in ie8 mode': cssContext({ 'plain component to complex shorthand': [ 'a{background:linear-gradient(to bottom,#000,#fff 4em) #000;background-color:#fff}', - 'a{background:linear-gradient(to bottom,#000,#fff 4em)#000;background-color:#fff}' + 'a{background:linear-gradient(to bottom,#000,#fff 4em) #000;background-color:#fff}' ], 'plain component to shorthand': [ 'a{background:url(bg.png) #000;background-color:#fff}', diff --git a/test/source-map-test.js b/test/source-map-test.js index 5dd56309..07f5c55a 100644 --- a/test/source-map-test.js +++ b/test/source-map-test.js @@ -1750,7 +1750,7 @@ vows.describe('source-map') return new CleanCSS({ sourceMap: true }).minify('a{background:url(image.png);background-color:#eee;background-repeat:repeat-x}'); }, 'has right output': function (minified) { - assert.equal(minified.styles, 'a{background:url(image.png) repeat-x #eee}'); + assert.equal(minified.styles, 'a{background:url(image.png)repeat-x #eee}'); }, 'has 5 mappings': function (minified) { assert.lengthOf(minified.sourceMap._mappings._array, 5); @@ -1780,7 +1780,7 @@ vows.describe('source-map') 'has a `repeat-x` mapping': function (minified) { var mapping = { generatedLine: 1, - generatedColumn: 28, + generatedColumn: 27, originalLine: 1, originalColumn: 68, source: '$stdin', @@ -1791,7 +1791,7 @@ vows.describe('source-map') 'has a `#eee` mapping': function (minified) { var mapping = { generatedLine: 1, - generatedColumn: 37, + generatedColumn: 36, originalLine: 1, originalColumn: 45, source: '$stdin', diff --git a/test/text/urls-processor-test.js b/test/text/urls-processor-test.js index c512a590..f2a75aaa 100644 --- a/test/text/urls-processor-test.js +++ b/test/text/urls-processor-test.js @@ -4,19 +4,19 @@ var UrlsProcessor = require('../../lib/text/urls-processor'); var lineBreak = require('os').EOL; -function processorContext(name, context, saveWaypoints, removeTrailingSpace) { +function processorContext(name, context, saveWaypoints) { var vowContext = {}; function escaped (expected) { return function (source) { - var escaped = new UrlsProcessor(null, saveWaypoints, removeTrailingSpace).escape(source); + var escaped = new UrlsProcessor(null, saveWaypoints).escape(source); assert.equal(escaped, expected); }; } function restored (expected) { return function (source) { - var processor = new UrlsProcessor(null, saveWaypoints, removeTrailingSpace); + var processor = new UrlsProcessor(null, saveWaypoints); var restored = processor.restore(processor.escape(source)); assert.equal(restored, expected); }; @@ -117,18 +117,4 @@ vows.describe(UrlsProcessor) ] }, true) ) - .addBatch( - processorContext('trailing space', { - 'removed': [ - 'div{background:url(some/file.png) repeat}', - 'div{background:__ESCAPED_URL_CLEAN_CSS0(0,18)__ repeat}', - 'div{background:url(some/file.png)repeat}' - ], - 'no space': [ - 'div{background:url(some/file.png)}', - 'div{background:__ESCAPED_URL_CLEAN_CSS0(0,18)__}', - 'div{background:url(some/file.png)}' - ] - }, true, true) - ) .export(module); -- 2.34.1