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.

diff --git a/History.md b/History.md
index 8c289de..607fb0f 100644 (file)
--- 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 fef0720..b68f940 100644 (file)
--- 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 d7d5167..fc65966 100644 (file)
--- 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 7290437..8e4fb96 100644 (file)
--- 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 533831a..6badb00 100644 (file)
--- 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}'