Fixes #1008 - edge case in breaking up font.

`font` shorthand needs `font-size` and `font-family`.
clean-css · Mar 5, 2018 · 913d72c · 913d72c
1 parent bedd8a9
commit 913d72c
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 2 deletions.
diff --git a/History.md b/History.md
@@ -4,6 +4,7 @@
 * Fixed issue [#988](https://github.com/jakubpawlowicz/clean-css/issues/988) - edge case in dropping default animation-duration.
 * Fixed issue [#989](https://github.com/jakubpawlowicz/clean-css/issues/989) - edge case in removing unused at rules.
 * Fixed issue [#1001](https://github.com/jakubpawlowicz/clean-css/issues/1001) - corrupted tokenizer state.
+* Fixed issue [#1008](https://github.com/jakubpawlowicz/clean-css/issues/1008) - edge case in breaking up `font` shorthand.
 
 [4.1.9 / 2017-09-19](https://github.com/jakubpawlowicz/clean-css/compare/v4.1.8...v4.1.9)
 ==================

diff --git a/lib/optimizer/level-2/break-up.js b/lib/optimizer/level-2/break-up.js
@@ -299,6 +299,10 @@ function font(property, compactable, validator) {
     return components;
   }
 
+  if (values.length < 2 || !_anyIsFontSize(values, validator) || !_anyIsFontFamily(values, validator)) {
+    throw new InvalidPropertyError('Invalid font values at ' + formatPosition(property.all[property.position][1][2][0]) + '. Ignoring.');
+  }
+
   if (values.length > 1 && _anyIsInherit(values)) {
     throw new InvalidPropertyError('Invalid font values at ' + formatPosition(values[0][2][0]) + '. Ignoring.');
   }
@@ -377,6 +381,36 @@ function font(property, compactable, validator) {
   return components;
 }
 
+function _anyIsFontSize(values, validator) {
+  var value;
+  var i, l;
+
+  for (i = 0, l = values.length; i < l; i++) {
+    value = values[i];
+
+    if (validator.isFontSizeKeyword(value[1]) || validator.isUnit(value[1]) && !validator.isDynamicUnit(value[1]) || validator.isFunction(value[1])) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+function _anyIsFontFamily(values, validator) {
+  var value;
+  var i, l;
+
+  for (i = 0, l = values.length; i < l; i++) {
+    value = values[i];
+
+    if (validator.isIdentifier(value[1])) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 function fourValues(property, compactable) {
   var componentNames = compactable[property.name].components;
   var components = [];

diff --git a/test/optimizer/level-2/break-up-test.js b/test/optimizer/level-2/break-up-test.js
@@ -1302,8 +1302,8 @@ vows.describe(breakUp)
           return _breakUp([
             [
               'property',
-              ['property-name', 'font'],
-              ['property-value', 'italic', [[0, 13, undefined]]],
+              ['property-name', 'font', [[0, 13, undefined]]],
+              ['property-value', 'italic'],
               ['property-value', 'sans-serif']
             ]
           ]);
@@ -1528,6 +1528,35 @@ vows.describe(breakUp)
           assert.deepEqual(components[6].value, [['property-value', '-clean-css-icon']]);
         }
       },
+      'normal as font': {
+        'topic': function () {
+          return _breakUp([
+            [
+              'property',
+              ['property-name', 'font', [[0, 6, undefined]]],
+              ['property-value', 'normal']
+            ]
+          ]);
+        },
+        'has 0 components': function (components) {
+          assert.lengthOf(components, 0);
+        }
+      },
+      'non-identifier as font family': {
+        'topic': function () {
+          return _breakUp([
+            [
+              'property',
+              ['property-name', 'font', [[0, 6, undefined]]],
+              ['property-value', '16px'],
+              ['property-value', '123']
+            ]
+          ]);
+        },
+        'has 0 components': function (components) {
+          assert.lengthOf(components, 0);
+        }
+      },
       'unset font': {
         'topic': function () {
           return _breakUp([