Only rewrite mq rules if necessary

cburgmer · Mar 26, 2015 · 125adce · 125adce
1 parent da273e9
commit 125adce
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 7 deletions.
diff --git a/src/mediaQueryHelper.js b/src/mediaQueryHelper.js
@@ -142,25 +142,46 @@ var mediaQueryHelper = (function (cssMediaQuery) {
     };
 
     var substituteEmWithPx = function (mediaQuery) {
-        var parsedQuery = cssMediaQuery.parse(mediaQuery);
+        var parsedQuery = cssMediaQuery.parse(mediaQuery),
+            hasChanges = false;
 
         parsedQuery.forEach(function (q) {
             q.expressions.forEach(function (exp) {
-                exp.value = replaceEmValueWithPx(exp.value);
+                var rewrittenValue = replaceEmValueWithPx(exp.value);
+
+                hasChanges |= rewrittenValue !== exp.value;
+                exp.value = rewrittenValue;
             });
         });
 
-        return module.serializeQuery(parsedQuery);
+        if (hasChanges) {
+            mediaQuery = module.serializeQuery(parsedQuery);
+        }
+
+        return mediaQuery;
     };
 
     var replaceEmsWithPx = function (mediaQueryRules) {
+        var anyRuleHasChanges = false;
+
         mediaQueryRules.forEach(function (rule) {
+            var hasChanges = false;
+
             var reworkedMediaQueries = asArray(rule.media).map(function (mediaQuery) {
-                return substituteEmWithPx(mediaQuery);
+                var rewrittenMediaQuery = substituteEmWithPx(mediaQuery);
+
+                hasChanges |= rewrittenMediaQuery !== mediaQuery;
+                return rewrittenMediaQuery;
             });
 
-            changeCssRule(rule, mediaQueryRule(reworkedMediaQueries, rule.cssRules));
+            if (hasChanges) {
+                changeCssRule(rule, mediaQueryRule(reworkedMediaQueries, rule.cssRules));
+            }
+
+            anyRuleHasChanges |= hasChanges;
         });
+
+        return anyRuleHasChanges;
     };
 
     module.workAroundWebKitEmSizeIssue = function (document) {
@@ -171,8 +192,10 @@ var mediaQueryHelper = (function (cssMediaQuery) {
                 return rule.type === window.CSSRule.MEDIA_RULE;
             });
 
-            replaceEmsWithPx(mediaQueryRules);
-            rewriteStyleContent(style);
+            var hasChanges = replaceEmsWithPx(mediaQueryRules);
+            if (hasChanges) {
+                rewriteStyleContent(style);
+            }
         });
     };
 

diff --git a/test/specs/MediaQueryHelperSpec.js b/test/specs/MediaQueryHelperSpec.js
@@ -86,6 +86,15 @@ describe("Media Query Helper", function () {
 
             expect(doc.querySelector('style').textContent).toMatch(/@media (all and )?\(min-width: 19.2px\)/);
         });
+
+        it("should not crash on errors by avoiding rewrite of rules", function () {
+            // Let's add an invalid media query and make sure it doesn't get changed (Firefox and Chrome complain)
+            addStyle('@media (min-width: 2) {}');
+
+            mediaQueryHelper.workAroundWebKitEmSizeIssue(doc);
+
+            expect(doc.querySelector('style').textContent).toMatch(/@media \(min-width: 2\)/);
+        });
     });
 
     // this functionality should sit inside css-mediaquery