From 1932189d277266e473b14c5fc9b7ef98e0b29edf Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Mon, 25 Aug 2014 13:43:29 +0100 Subject: [PATCH] Fixes #339 - skips invalid properties. * Skips processing of properties without values. * Passing selector name to property optimiser is far from ideal but have to do for the time being. --- History.md | 1 + lib/properties/optimizer.js | 21 ++++++++++++++------- lib/selectors/optimizer.js | 8 ++++---- test/module-test.js | 17 +++++++++++++++++ test/unit-test.js | 10 +++++++++- 5 files changed, 45 insertions(+), 12 deletions(-) diff --git a/History.md b/History.md index 8c289de2..607fb0f6 100644 --- a/History.md +++ b/History.md @@ -2,6 +2,7 @@ ================== * Makes multival operations idempotent. +* Fixed issue [#339](https://github.com/GoalSmashers/clean-css/issues/339) - skips invalid properties. * Fixed issue [#341](https://github.com/GoalSmashers/clean-css/issues/341) - ensure output is shorter than input. [2.2.13 / 2014-08-12](https://github.com/jakubpawlowicz/clean-css/compare/v2.2.12...v2.2.13) diff --git a/lib/properties/optimizer.js b/lib/properties/optimizer.js index fef0720d..b68f940c 100644 --- a/lib/properties/optimizer.js +++ b/lib/properties/optimizer.js @@ -3,7 +3,7 @@ var processableInfo = require('./processable'); var overrideCompactor = require('./override-compactor'); var shorthandCompactor = require('./shorthand-compactor'); -module.exports = function Optimizer(compatibility, aggressiveMerging) { +module.exports = function Optimizer(compatibility, aggressiveMerging, context) { var overridable = { 'animation-delay': ['animation'], 'animation-direction': ['animation'], @@ -116,11 +116,11 @@ module.exports = function Optimizer(compatibility, aggressiveMerging) { } } - var tokenize = function(body) { + var tokenize = function(body, selector) { var tokens = body.split(';'); var keyValues = []; - if (tokens.length === 0 || (tokens.length == 1 && tokens[0].indexOf(IE_BACKSLASH_HACK) == -1)) + if (tokens.length === 0 || (tokens.length == 1 && tokens[0].indexOf(IE_BACKSLASH_HACK) == -1 && tokens[0][tokens[0].length - 1] != ':')) return; for (var i = 0, l = tokens.length; i < l; i++) { @@ -129,9 +129,16 @@ module.exports = function Optimizer(compatibility, aggressiveMerging) { continue; var firstColon = token.indexOf(':'); + var property = token.substring(0, firstColon); + var value = token.substring(firstColon + 1); + if (value === '') { + context.warnings.push('Empty property \'' + property + '\' inside \'' + selector + '\' selector. Ignoring.'); + continue; + } + keyValues.push([ - token.substring(0, firstColon), - token.substring(firstColon + 1), + property, + value, token.indexOf('!important') > -1, token.indexOf(IE_BACKSLASH_HACK, firstColon + 1) === token.length - IE_BACKSLASH_HACK.length ]); @@ -257,10 +264,10 @@ module.exports = function Optimizer(compatibility, aggressiveMerging) { }; return { - process: function(body, allowAdjacent, skipCompacting) { + process: function(body, allowAdjacent, skipCompacting, selector) { var result = body; - var tokens = tokenize(body); + var tokens = tokenize(body, selector); if (tokens) { var optimized = optimize(tokens, allowAdjacent); result = rebuild(optimized); diff --git a/lib/selectors/optimizer.js b/lib/selectors/optimizer.js index d7d5167b..fc65966b 100644 --- a/lib/selectors/optimizer.js +++ b/lib/selectors/optimizer.js @@ -10,7 +10,7 @@ module.exports = function Optimizer(data, context, options) { var minificationsMade = []; - var propertyOptimizer = new PropertyOptimizer(options.compatibility, options.aggressiveMerging); + var propertyOptimizer = new PropertyOptimizer(options.compatibility, options.aggressiveMerging, context); var cleanUpSelector = function(selectors) { if (selectors.indexOf(',') == -1) @@ -105,7 +105,7 @@ module.exports = function Optimizer(data, context, options) { if (token.selector == lastToken.selector) { var joinAt = [lastToken.body.split(';').length]; - lastToken.body = propertyOptimizer.process(lastToken.body + ';' + token.body, joinAt); + lastToken.body = propertyOptimizer.process(lastToken.body + ';' + token.body, joinAt, false, token.selector); forRemoval.push(i); } else if (token.body == lastToken.body && !isSpecial(token.selector) && !isSpecial(lastToken.selector)) { lastToken.selector = cleanUpSelector(lastToken.selector + ',' + token.selector); @@ -253,7 +253,7 @@ module.exports = function Optimizer(data, context, options) { joinsAt.push((joinsAt[j - 1] || 0) + splitBodies[j].length); } - var optimizedBody = propertyOptimizer.process(bodies.join(';'), joinsAt, true); + var optimizedBody = propertyOptimizer.process(bodies.join(';'), joinsAt, true, selector); var optimizedProperties = optimizedBody.split(';'); var processedCount = processedTokens.length; @@ -286,7 +286,7 @@ module.exports = function Optimizer(data, context, options) { if (token.selector) { token.selector = cleanUpSelector(token.selector); - token.body = propertyOptimizer.process(token.body, false); + token.body = propertyOptimizer.process(token.body, false, false, token.selector); } else if (token.block) { optimize(token.body); } diff --git a/test/module-test.js b/test/module-test.js index 72904374..8e4fb96b 100644 --- a/test/module-test.js +++ b/test/module-test.js @@ -151,6 +151,23 @@ vows.describe('module tests').addBatch({ assert.equal(minifier.warnings[0], 'Unexpected content: \'color:#535353}\'. Ignoring.'); } }, + 'warnings on invalid properties': { + topic: function() { + var minifier = new CleanCSS(); + var minified = minifier.minify('a{color:}'); + this.callback(null, minified, minifier); + }, + 'should minify correctly': function(error, minified) { + assert.equal(minified, ''); + }, + 'should raise no errors': function(error, minified, minifier) { + assert.equal(minifier.errors.length, 0); + }, + 'should raise one warning': function(error, minified, minifier) { + assert.equal(minifier.warnings.length, 1); + assert.equal(minifier.warnings[0], 'Empty property \'color\' inside \'a\' selector. Ignoring.'); + } + }, 'no errors': { topic: function() { var minifier = new CleanCSS(); diff --git a/test/unit-test.js b/test/unit-test.js index 533831ab..6badb00e 100644 --- a/test/unit-test.js +++ b/test/unit-test.js @@ -2062,7 +2062,15 @@ title']{display:block}", 'p{background:red;background-image:url(1.png),url(2.png)}', 'p{background:url(1.png),url(2.png) red}' ], - 'unknown @ rule': '@unknown "test";h1{color:red}' + 'unknown @ rule': '@unknown "test";h1{color:red}', + 'property without a value': [ + 'a{color:}', + '' + ], + 'properties without values': [ + 'a{padding:;border-radius: ;background:red}', + 'a{background:red}' + ] }), 'viewport units': cssContext({ 'shorthand margin with viewport width not changed': 'div{margin:5vw}' -- 2.34.1