From 4054bb13cd955b8fa798ffd70056941a0e0bfeba Mon Sep 17 00:00:00 2001
From: mzabriskie <mzabriskie@gmail.com>
Date: Mon, 26 Jan 2015 17:33:44 -0700
Subject: [PATCH] Provide warning when using styles containing a semicolon

---
 src/browser/ui/dom/CSSPropertyOperations.js   | 47 ++++++++++++++-----
 .../__tests__/CSSPropertyOperations-test.js   | 10 ++++
 2 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/src/browser/ui/dom/CSSPropertyOperations.js b/src/browser/ui/dom/CSSPropertyOperations.js
index f40ce5f213..2b8cf0f004 100644
--- a/src/browser/ui/dom/CSSPropertyOperations.js
+++ b/src/browser/ui/dom/CSSPropertyOperations.js
@@ -37,7 +37,11 @@ if (__DEV__) {
   // 'msTransform' is correct, but the other prefixes should be capitalized
   var badVendoredStyleNamePattern = /^(?:webkit|moz|o)[A-Z]/;
 
+  // style values shouldn't contain a semicolon
+  var badStyleValueWithSemicolonPattern = /;(\s*)$/;
+
   var warnedStyleNames = {};
+  var warnedStyleValues = {};
 
   var warnHyphenatedStyleName = function(name) {
     if (warnedStyleNames.hasOwnProperty(name) && warnedStyleNames[name]) {
@@ -64,6 +68,33 @@ if (__DEV__) {
       name.charAt(0).toUpperCase() + name.slice(1) + '?'
     );
   };
+
+  var warnStyleValueWithSemicolon = function(name, value) {
+    if (warnedStyleValues.hasOwnProperty(value) && warnedStyleValues[value]) {
+      return;
+    }
+    
+    warnedStyleValues[value] = true;
+    warning(
+      false,
+      'Style property values shouldn\'t contain a semicolon. Try "' +
+      name + ': ' + value.replace(badStyleValueWithSemicolonPattern, '') + '" instead.'
+    );
+  };
+
+  /**
+   * @param {string} name
+   * @param {string|number} value
+   */
+  var assertValidStyle = function(name, value) {
+    if (name.indexOf('-') > -1) {
+      warnHyphenatedStyleName(name);
+    } else if (badVendoredStyleNamePattern.test(name)) {
+      warnBadVendoredStyleName(name);
+    } else if (badStyleValueWithSemicolonPattern.test(value)) {
+      warnStyleValueWithSemicolon(name, value);
+    }
+  };
 }
 
 /**
@@ -89,14 +120,10 @@ var CSSPropertyOperations = {
       if (!styles.hasOwnProperty(styleName)) {
         continue;
       }
-      if (__DEV__) {
-        if (styleName.indexOf('-') > -1) {
-          warnHyphenatedStyleName(styleName);
-        } else if (badVendoredStyleNamePattern.test(styleName)) {
-          warnBadVendoredStyleName(styleName);
-        }
-      }
       var styleValue = styles[styleName];
+      if (__DEV__) {
+        assertValidStyle(styleName, styleValue);
+      }
       if (styleValue != null) {
         serialized += processStyleName(styleName) + ':';
         serialized += dangerousStyleValue(styleName, styleValue) + ';';
@@ -119,11 +146,7 @@ var CSSPropertyOperations = {
         continue;
       }
       if (__DEV__) {
-        if (styleName.indexOf('-') > -1) {
-          warnHyphenatedStyleName(styleName);
-        } else if (badVendoredStyleNamePattern.test(styleName)) {
-          warnBadVendoredStyleName(styleName);
-        }
+        assertValidStyle(styleName, styles[styleName]); 
       }
       var styleValue = dangerousStyleValue(styleName, styles[styleName]);
       if (styleName === 'float') {
diff --git a/src/browser/ui/dom/__tests__/CSSPropertyOperations-test.js b/src/browser/ui/dom/__tests__/CSSPropertyOperations-test.js
index e7f0519138..1fec0e39b5 100644
--- a/src/browser/ui/dom/__tests__/CSSPropertyOperations-test.js
+++ b/src/browser/ui/dom/__tests__/CSSPropertyOperations-test.js
@@ -153,4 +153,14 @@ describe('CSSPropertyOperations', function() {
     expect(console.warn.argsForCall[1][0]).toContain('WebkitTransform');
   });
 
+  it('should warn about style having a trailing semicolon', function() {
+    spyOn(console, 'warn');
+
+    CSSPropertyOperations.createMarkupForStyles({
+      backgroundColor: 'blue;'
+    });
+
+    expect(console.warn.callCount).toBe(1);
+    expect(console.warn.argsForCall[0][0]).toContain('Try "backgroundColor: blue" instead');
+  });
 });