Fix handling of media-type-only media queries

@media print {...} (& similar) were making us crash.
twbs · Jan 19, 2015 · 9d035dd · 9d035dd
1 parent 2ee4471
commit 9d035dd
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 15 deletions.
diff --git a/src/nodejs/postprocessor.js b/src/nodejs/postprocessor.js
@@ -12,21 +12,27 @@ var postcss = require('postcss');
 var mediaQuery = require('css-mediaquery');
 
 
-// Checks whether the at-rule is: @media (hover: hover) {...}
-function isSimpleMediaHoverHover(atRule) {
+// Returns media type iff the at-rule is: @media optional-media-type (hover: hover) {...}
+function mediaTypeIfSimpleHoverHover(atRule) {
     var mediaOrs = mediaQuery.parse(atRule.params);
-    if (mediaOrs.length > 1) {
+    if (mediaOrs.length !== 1) {
         return false;
     }
     var mediaAnds = mediaOrs[0];
     if (mediaAnds.inverse) {
         return false;
     }
-    if (mediaAnds.expressions.length > 1) {
+    if (mediaAnds.expressions.length !== 1) {
         return false;
     }
+
     var mediaExpr = mediaAnds.expressions[0];
-    return mediaExpr.feature === 'hover' && mediaExpr.value === 'hover';
+    if (mediaExpr.feature === 'hover' && mediaExpr.value === 'hover') {
+        return mediaAnds.type;
+    }
+    else {
+        return undefined;
+    }
 }
 
 function replaceWithItsChildren(atRule) {
@@ -69,14 +75,39 @@ module.exports = postcss(function process(css, opts) {
     }
 
     css.eachAtRule('media', function (atRule) {
-        if (!isSimpleMediaHoverHover(atRule)) {
-            return;
-        }
+        var mediaType = mediaTypeIfSimpleHoverHover(atRule);
+        switch (mediaType) {
+            case 'all':
+                /* falls through */
+            case 'screen': {
+                atRule.eachRule(function (rule) {
+                    prefixSelectorsWith(rule, hoverSelectorPrefix);
+                });
+                if (mediaType === 'screen') {
+                    atRule.params = 'screen';
+                }
+                else {
+                    // Remove tautological @media all {...} wrapper
+                    replaceWithItsChildren(atRule);
+                }
+                return;
+            }
 
-        atRule.eachRule(function (rule) {
-            prefixSelectorsWith(rule, hoverSelectorPrefix);
-        });
+            case 'print':
+                /* falls through */
+            case 'speech': {
+                // These media types never support hovering
+                // Delete always-false media query
+                atRule.removeSelf();
+                return;
+            }
 
-        replaceWithItsChildren(atRule);
+            case undefined: {
+                return; // Media query irrelevant or too complicated
+            }
+            default: {
+                return; // Deprecated media type; take no action.
+            }
+        }
     });
 });
diff --git a/test/postprocessor_test.js b/test/postprocessor_test.js
@@ -41,20 +41,36 @@ exports.mq4HoverShim = {
         );
         test.done();
     },
-    'skips media queries with ORs': function (test) {
+    'skips media queries with only a media type': function (test) {
         test.expect(1);
+        test.deepEqual(
+           postprocessor.process("@media screen { .foobar { display: none; } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
+           "@media screen { .foobar { display: none; } }"
+        );
+        test.done();
+    },
+    'skips media queries with ORs': function (test) {
+        test.expect(2);
         test.deepEqual(
             postprocessor.process("@media (hover: hover), (orientation: landscape) { .foobar { display: none; } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
             "@media (hover: hover), (orientation: landscape) { .foobar { display: none; } }"
         );
+        test.deepEqual(
+            postprocessor.process("@media screen (hover: hover), (orientation: landscape) { .foobar { display: none; } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
+            "@media screen (hover: hover), (orientation: landscape) { .foobar { display: none; } }"
+        );
         test.done();
     },
     'skips media queries with ANDs': function (test) {
-        test.expect(1);
+        test.expect(2);
         test.deepEqual(
             postprocessor.process("@media (hover: hover) and (orientation: landscape) { .foobar { display: none; } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
             "@media (hover: hover) and (orientation: landscape) { .foobar { display: none; } }"
         );
+        test.deepEqual(
+            postprocessor.process("@media screen (hover: hover) and (orientation: landscape) { .foobar { display: none; } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
+            "@media screen (hover: hover) and (orientation: landscape) { .foobar { display: none; } }"
+        );
         test.done();
     },
     'skips media queries that are not about the hover media feature': function (test) {
@@ -86,15 +102,39 @@ exports.mq4HoverShim = {
         test.done();
     },
     'handles nested at-rules': function (test) {
-        test.expect(2);
+        test.expect(4);
         test.deepEqual(
             postprocessor.process("@media (orientation: landscape) { @media (hover: hover) { .foobar { display: none; } } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
             "@media (orientation: landscape) { PREFIX>.foobar { display: none; } }"
         );
+        test.deepEqual(
+            postprocessor.process("@media screen (orientation: landscape) { @media (hover: hover) { .foobar { display: none; } } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
+            "@media screen (orientation: landscape) { PREFIX>.foobar { display: none; } }"
+        );
         test.deepEqual(
             postprocessor.process("@media (hover: hover) { @media (orientation: landscape) { .foobar { display: none; } } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
             "@media (orientation: landscape) {\n    PREFIX>.foobar {\n        display: none;\n    }\n}"
         );
+        test.deepEqual(
+            postprocessor.process("@media (hover: hover) { @media screen (orientation: landscape) { .foobar { display: none; } } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
+            "@media screen (orientation: landscape) {\n    PREFIX>.foobar {\n        display: none;\n    }\n}"
+        );
+        test.done();
+    },
+    'handles applicable media types': function (test) {
+        test.expect(1);
+        test.deepEqual(
+            postprocessor.process("@media screen (hover: hover) { .foobar { display: none; } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
+            "@media screen { PREFIX>.foobar { display: none; } }"
+        );
+        test.done();
+    },
+    'handles non-applicable media types': function (test) {
+        test.expect(1);
+        test.deepEqual(
+            postprocessor.process("@media print (hover: hover) { .foobar { display: none; } }", {hoverSelectorPrefix: 'PREFIX>'}).css,
+            ""
+        );
         test.done();
     },
     'errors when hoverSelectorPrefix is not provided': function (test) {