Fix merging rules with vendor prefixes

Fixes parcel-bundler#478

Author: devongovett

src/lib.rs

@@ -9250,6 +9250,91 @@ mod tests {
         ..Browsers::default()
       },
     );
+
+    prefix_test(
+      r#"
+      .foo:placeholder-shown .bar { color: red; }
+      .foo:autofill .baz { color: red; }
+      "#,
+      indoc! {r#"
+      .foo:placeholder-shown .bar, .foo:-webkit-autofill .baz {
+        color: red;
+      }
+
+      .foo:placeholder-shown .bar, .foo:autofill .baz {
+        color: red;
+      }
+      "#},
+      Browsers {
+        chrome: Some(103 << 16),
+        ..Browsers::default()
+      },
+    );
+
+    prefix_test(
+      r#"
+      .foo:placeholder-shown .bar,.foo:autofill .baz{color:red}
+      "#,
+      indoc! {r#"
+      .foo:placeholder-shown .bar, .foo:-webkit-autofill .baz {
+        color: red;
+      }
+
+      .foo:placeholder-shown .bar, .foo:autofill .baz {
+        color: red;
+      }
+      "#},
+      Browsers {
+        chrome: Some(103 << 16),
+        ..Browsers::default()
+      },
+    );
+
+    prefix_test(
+      r#"
+      .foo:placeholder-shown .bar, .foo:-webkit-autofill .baz {
+        color: red;
+      }
+
+      .foo:placeholder-shown .bar, .foo:autofill .baz {
+        color: red;
+      }
+      "#,
+      indoc! {r#"
+      .foo:placeholder-shown .bar, .foo:-webkit-autofill .baz {
+        color: red;
+      }
+
+      .foo:placeholder-shown .bar, .foo:autofill .baz {
+        color: red;
+      }
+      "#},
+      Browsers {
+        chrome: Some(103 << 16),
+        ..Browsers::default()
+      },
+    );
+
+    test(
+      r#"
+      .foo:placeholder-shown .bar, .foo:-webkit-autofill .baz {
+        color: red;
+      }
+
+      .foo:placeholder-shown .bar, .foo:autofill .baz {
+        color: red;
+      }
+      "#,
+      indoc! {r#"
+      .foo:placeholder-shown .bar, .foo:-webkit-autofill .baz {
+        color: red;
+      }
+
+      .foo:placeholder-shown .bar, .foo:autofill .baz {
+        color: red;
+      }
+      "#},
+    );
   }
 
   #[test]

src/rules/mod.rs

@@ -587,11 +587,9 @@ impl<'i, T> CssRuleList<'i, T> {
             continue;
           }
 
-          if context.targets.should_compile_selectors() {
-            style.vendor_prefix = get_prefix(&style.selectors);
-            if style.vendor_prefix.contains(VendorPrefix::None) {
-              style.vendor_prefix = downlevel_selectors(style.selectors.0.as_mut_slice(), *context.targets);
-            }
+          style.vendor_prefix = get_prefix(&style.selectors);
+          if style.vendor_prefix.contains(VendorPrefix::None) & context.targets.should_compile_selectors() {
+            style.vendor_prefix = downlevel_selectors(style.selectors.0.as_mut_slice(), *context.targets);
           }
 
           // Attempt to merge the new rule with the last rule we added.
@@ -724,28 +722,28 @@ fn merge_style_rules<'i, T>(
     && style.rules.0.is_empty()
     && last_style_rule.rules.0.is_empty()
   {
-    // Append the selectors to the last rule if the declarations are the same, and all selectors are compatible.
-    if style.is_compatible(context.targets.browsers) && last_style_rule.is_compatible(context.targets.browsers) {
-      last_style_rule.selectors.0.extend(style.selectors.0.drain(..));
-      return true;
-    }
-
     // If both selectors are potentially vendor prefixable, and they are
     // equivalent minus prefixes, add the prefix to the last rule.
     if !style.vendor_prefix.is_empty()
       && !last_style_rule.vendor_prefix.is_empty()
-      && !last_style_rule.vendor_prefix.contains(style.vendor_prefix)
       && is_equivalent(&style.selectors, &last_style_rule.selectors)
     {
       // If the new rule is unprefixed, replace the prefixes of the last rule.
       // Otherwise, add the new prefix.
-      if style.vendor_prefix.contains(VendorPrefix::None) {
+      if style.vendor_prefix.contains(VendorPrefix::None) && context.targets.should_compile_selectors() {
         last_style_rule.vendor_prefix = style.vendor_prefix;
       } else {
         last_style_rule.vendor_prefix |= style.vendor_prefix;
       }
       return true;
     }
+
+    // Append the selectors to the last rule if the declarations are the same, and all selectors are compatible.
+    if style.is_compatible(context.targets.browsers) && last_style_rule.is_compatible(context.targets.browsers) {
+      last_style_rule.selectors.0.extend(style.selectors.0.drain(..));
+      last_style_rule.vendor_prefix |= style.vendor_prefix;
+      return true;
+    }
   }
   false
 }

src/rules/style.rs

@@ -193,30 +193,19 @@ impl<'a, 'i, T: ToCss> ToCss for StyleRule<'i, T> {
       self.to_css_base(dest)
     } else {
       let mut first_rule = true;
-      macro_rules! prefix {
-        ($prefix: ident) => {
-          if self.vendor_prefix.contains(VendorPrefix::$prefix) {
-            #[allow(unused_assignments)]
-            if first_rule {
-              first_rule = false;
-            } else {
-              if !dest.minify {
-                dest.write_char('\n')?; // no indent
-              }
-              dest.newline()?;
-            }
-            dest.vendor_prefix = VendorPrefix::$prefix;
-            self.to_css_base(dest)?;
+      for prefix in self.vendor_prefix {
+        if first_rule {
+          first_rule = false;
+        } else {
+          if !dest.minify {
+            dest.write_char('\n')?; // no indent
           }
-        };
+          dest.newline()?;
+        }
+        dest.vendor_prefix = prefix;
+        self.to_css_base(dest)?;
       }
 
-      prefix!(WebKit);
-      prefix!(Moz);
-      prefix!(Ms);
-      prefix!(O);
-      prefix!(None);
-
       dest.vendor_prefix = VendorPrefix::empty();
       Ok(())
     }

src/selector.rs

@@ -606,7 +606,7 @@ where
       dest.write_char(':')?;
       // If the printer has a vendor prefix override, use that.
       let vp = if !dest.vendor_prefix.is_empty() {
-        dest.vendor_prefix
+        (dest.vendor_prefix & *$prefix).or_none()
       } else {
         *$prefix
       };
@@ -658,7 +658,7 @@ where
     Fullscreen(prefix) => {
       dest.write_char(':')?;
       let vp = if !dest.vendor_prefix.is_empty() {
-        dest.vendor_prefix
+        (dest.vendor_prefix & *prefix).or_none()
       } else {
         *prefix
       };
@@ -766,20 +766,21 @@ impl<'i> PseudoClass<'i> {
     }
   }
 
-  pub(crate) fn get_necessary_prefixes(&self, targets: Targets) -> VendorPrefix {
+  pub(crate) fn get_necessary_prefixes(&mut self, targets: Targets) -> VendorPrefix {
     use crate::prefixes::Feature;
     use PseudoClass::*;
-    let feature = match self {
-      Fullscreen(p) if *p == VendorPrefix::None => Feature::PseudoClassFullscreen,
-      AnyLink(p) if *p == VendorPrefix::None => Feature::PseudoClassAnyLink,
-      ReadOnly(p) if *p == VendorPrefix::None => Feature::PseudoClassReadOnly,
-      ReadWrite(p) if *p == VendorPrefix::None => Feature::PseudoClassReadWrite,
-      PlaceholderShown(p) if *p == VendorPrefix::None => Feature::PseudoClassPlaceholderShown,
-      Autofill(p) if *p == VendorPrefix::None => Feature::PseudoClassAutofill,
+    let (p, feature) = match self {
+      Fullscreen(p) => (p, Feature::PseudoClassFullscreen),
+      AnyLink(p) => (p, Feature::PseudoClassAnyLink),
+      ReadOnly(p) => (p, Feature::PseudoClassReadOnly),
+      ReadWrite(p) => (p, Feature::PseudoClassReadWrite),
+      PlaceholderShown(p) => (p, Feature::PseudoClassPlaceholderShown),
+      Autofill(p) => (p, Feature::PseudoClassAutofill),
       _ => return VendorPrefix::empty(),
     };
 
-    targets.prefixes(VendorPrefix::None, feature)
+    *p = targets.prefixes(*p, feature);
+    *p
   }
 }
 
@@ -1001,7 +1002,7 @@ where
       dest.write_str("::")?;
       // If the printer has a vendor prefix override, use that.
       let vp = if !dest.vendor_prefix.is_empty() {
-        dest.vendor_prefix
+        (dest.vendor_prefix & *$prefix).or_none()
       } else {
         *$prefix
       };
@@ -1169,18 +1170,19 @@ impl<'i> PseudoElement<'i> {
     }
   }
 
-  pub(crate) fn get_necessary_prefixes(&self, targets: Targets) -> VendorPrefix {
+  pub(crate) fn get_necessary_prefixes(&mut self, targets: Targets) -> VendorPrefix {
     use crate::prefixes::Feature;
     use PseudoElement::*;
-    let feature = match self {
-      Selection(p) if *p == VendorPrefix::None => Feature::PseudoElementSelection,
-      Placeholder(p) if *p == VendorPrefix::None => Feature::PseudoElementPlaceholder,
-      Backdrop(p) if *p == VendorPrefix::None => Feature::PseudoElementBackdrop,
-      FileSelectorButton(p) if *p == VendorPrefix::None => Feature::PseudoElementFileSelectorButton,
+    let (p, feature) = match self {
+      Selection(p) => (p, Feature::PseudoElementSelection),
+      Placeholder(p) => (p, Feature::PseudoElementPlaceholder),
+      Backdrop(p) => (p, Feature::PseudoElementBackdrop),
+      FileSelectorButton(p) => (p, Feature::PseudoElementFileSelectorButton),
       _ => return VendorPrefix::empty(),
     };
 
-    targets.prefixes(VendorPrefix::None, feature)
+    *p = targets.prefixes(*p, feature);
+    *p
   }
 }
 
@@ -1835,8 +1837,10 @@ pub(crate) fn get_prefix(selectors: &SelectorList) -> VendorPrefix {
       };
 
       if !p.is_empty() {
-        if prefix.is_empty() || prefix == p {
-          prefix = p;
+        // Allow none to be mixed with a prefix.
+        let prefix_without_none = prefix - VendorPrefix::None;
+        if prefix_without_none.is_empty() || prefix_without_none == p {
+          prefix |= p;
         } else {
           return VendorPrefix::empty();
         }
@@ -1856,10 +1860,6 @@ const RTL_LANGS: &[&str] = &[
 /// Returns the necessary vendor prefixes.
 pub(crate) fn downlevel_selectors(selectors: &mut [Selector], targets: Targets) -> VendorPrefix {
   let mut necessary_prefixes = VendorPrefix::empty();
-  if !targets.should_compile_selectors() {
-    return necessary_prefixes;
-  }
-
   for selector in selectors {
     for component in selector.iter_mut_raw_match_order() {
       necessary_prefixes |= downlevel_component(component, targets);