From: Jakub Pawlowicz <contact@jakubpawlowicz.com>
Date: Mon, 20 Oct 2014 19:15:27 +0000 (+0100)
Subject: Fixes #363 - `rem` units overriding more understandable ones.
X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=13e6752e20dbeee45528aa11353a1020ba9a89f3;p=clean-css.git

Fixes #363 - `rem` units overriding more understandable ones.
---

diff --git a/History.md b/History.md
index 765dfb42..e725c519 100644
--- a/History.md
+++ b/History.md
@@ -14,6 +14,7 @@
 * Speeds up advanced processing by shortening optimize loop.
 * Fixed issue [#344](https://github.com/GoalSmashers/clean-css/issues/344) - merging background-size into shorthand.
 * Fixed issue [#360](https://github.com/GoalSmashers/clean-css/issues/360) - adds 7 extra CSS colors.
+* Fixed issue [#363](https://github.com/GoalSmashers/clean-css/issues/363) - `rem` units overriding `px`.
 
 [2.2.16 / 2014-09-16](https://github.com/jakubpawlowicz/clean-css/compare/v2.2.15...v2.2.16)
 ==================
diff --git a/lib/properties/optimizer.js b/lib/properties/optimizer.js
index 9f4fbc16..83609927 100644
--- a/lib/properties/optimizer.js
+++ b/lib/properties/optimizer.js
@@ -102,6 +102,7 @@ module.exports = function Optimizer(compatibility, aggressiveMerging, context) {
   };
 
   var IE_BACKSLASH_HACK = '\\9';
+  var processable = processableInfo.processable(compatibility);
 
   var overrides = {};
   for (var granular in overridable) {
@@ -204,7 +205,7 @@ module.exports = function Optimizer(compatibility, aggressiveMerging, context) {
           if (compatibility.properties.ieSuffixHack && !wasIEHack && isIEHack)
             break;
 
-          var _info = processableInfo.processable[_property];
+          var _info = processable[_property];
           if (!isIEHack && !wasIEHack && _info && _info.canOverride && !_info.canOverride(tokens[toOverridePosition][1], value))
             break;
 
@@ -244,7 +245,6 @@ module.exports = function Optimizer(compatibility, aggressiveMerging, context) {
   };
 
   var compact = function (input) {
-    var processable = processableInfo.processable;
     var Token = processableInfo.Token;
 
     var tokens = Token.tokenize(input);
diff --git a/lib/properties/processable.js b/lib/properties/processable.js
index 8211e49a..bc6358c8 100644
--- a/lib/properties/processable.js
+++ b/lib/properties/processable.js
@@ -38,6 +38,8 @@ module.exports = (function () {
       // NOTE: there is no point in having different vendor-specific functions override each other or standard functions,
       //       or having standard functions override vendor-specific functions, but standard functions can override each other
       // NOTE: vendor-specific property values are not taken into consideration here at the moment
+      if (validator.isValidAndCompatibleUnitWithoutFunction(val1) && !validator.isValidAndCompatibleUnitWithoutFunction(val2))
+        return false;
 
       if (validator.isValidUnitWithoutFunction(val2))
         return true;
@@ -859,7 +861,12 @@ module.exports = (function () {
 
   return {
     implementedFor: /background|border|color|list|margin|outline|padding|transform/,
-    processable: processable,
+    processable: function (compatibility) {
+      // FIXME: we need a proper OO way
+      validator.setCompatibility(compatibility);
+
+      return processable;
+    },
     Token: Token
   };
 })();
diff --git a/lib/properties/validator.js b/lib/properties/validator.js
index 6c766d32..0bfe3139 100644
--- a/lib/properties/validator.js
+++ b/lib/properties/validator.js
@@ -6,7 +6,8 @@ var Splitter = require('../utils/splitter');
 module.exports = (function () {
   // Regexes used for stuff
   var widthKeywords = ['thin', 'thick', 'medium', 'inherit', 'initial'];
-  var cssUnitRegexStr = '(\\-?\\.?\\d+\\.?\\d*(px|%|em|rem|in|cm|mm|ex|pt|pc|vw|vh|vmin|vmax|)|auto|inherit)';
+  var allUnits = ['px', '%', 'em', 'rem', 'in', 'cm', 'mm', 'ex', 'pt', 'pc', 'vw', 'vh', 'vmin', 'vmax'];
+  var cssUnitRegexStr = '(\\-?\\.?\\d+\\.?\\d*(' + allUnits.join('|') + '|)|auto|inherit)';
   var cssFunctionNoVendorRegexStr = '[A-Z]+(\\-|[A-Z]|[0-9])+\\(([A-Z]|[0-9]|\\ |\\,|\\#|\\+|\\-|\\%|\\.|\\(|\\))*\\)';
   var cssFunctionVendorRegexStr = '\\-(\\-|[A-Z]|[0-9])+\\(([A-Z]|[0-9]|\\ |\\,|\\#|\\+|\\-|\\%|\\.|\\(|\\))*\\)';
   var cssVariableRegexStr = 'var\\(\\-\\-[^\\)]+\\)';
@@ -28,7 +29,27 @@ module.exports = (function () {
   var listStylePositionKeywords = ['inside', 'outside', 'inherit'];
   var outlineStyleKeywords = ['auto', 'inherit', 'hidden', 'none', 'dotted', 'dashed', 'solid', 'double', 'groove', 'ridge', 'inset', 'outset'];
 
+  var compatibleCssUnitRegex;
+  var compatibleCssUnitAnyRegex;
+
   var validator = {
+    // FIXME: we need a proper OO here
+    setCompatibility: function (compatibility) {
+      if (compatibility.units.rem) {
+        compatibleCssUnitRegex = cssUnitRegex;
+        compatibleCssUnitAnyRegex = cssUnitAnyRegex;
+        return;
+      }
+
+      var validUnits = allUnits.slice(0).filter(function (value) {
+        return value != 'rem';
+      });
+
+      var compatibleCssUnitRegexStr = '(\\-?\\.?\\d+\\.?\\d*(' + validUnits.join('|') + ')|auto|inherit)';
+      compatibleCssUnitRegex = new RegExp('^' + compatibleCssUnitRegexStr + '$', 'i');
+      compatibleCssUnitAnyRegex = new RegExp('^(none|' + widthKeywords.join('|') + '|' + compatibleCssUnitRegexStr + '|' + cssVariableRegexStr + '|' + cssFunctionNoVendorRegexStr + '|' + cssFunctionVendorRegexStr + ')$', 'i');
+    },
+
     isValidHexColor: function (s) {
       return (s.length === 4 || s.length === 7) && s[0] === '#';
     },
@@ -60,6 +81,12 @@ module.exports = (function () {
     isValidUnitWithoutFunction: function (s) {
       return cssUnitRegex.test(s);
     },
+    isValidAndCompatibleUnit: function (s) {
+      return compatibleCssUnitAnyRegex.test(s);
+    },
+    isValidAndCompatibleUnitWithoutFunction: function (s) {
+      return compatibleCssUnitRegex.test(s);
+    },
     isValidFunctionWithoutVendorPrefix: function (s) {
       return cssFunctionNoVendorRegex.test(s);
     },
diff --git a/test/integration-test.js b/test/integration-test.js
index 708ba77f..4eb706a5 100644
--- a/test/integration-test.js
+++ b/test/integration-test.js
@@ -1734,6 +1734,9 @@ title']{display:block}",
     'of supported and unsupported selector': '.one{color:red}.two:last-child{color:red}',
     'of two unsupported selectors': '.one:before{color:red}.two:last-child{color:red}'
   }, { compatibility: 'ie7' }),
+  'units - IE8 compatibility': cssContext({
+    'rems': 'div{padding-top:16px;padding-top:1rem}'
+  }, { compatibility: 'ie8' }),
   'redefined more granular properties': redefineContext({
     'animation-delay': ['animation'],
     'animation-direction': ['animation'],