Skip to content

Commit

Permalink
Ensure fallback rules for logical properties are before nested rules
Browse files Browse the repository at this point in the history
Fixes #175

Also ensures that nested rules use a different compilation context from parents so that logical rules are not merged.
  • Loading branch information
devongovett committed Jun 12, 2023
1 parent 11f4179 commit e66d99c
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 7 deletions.
12 changes: 12 additions & 0 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ impl<'i, 'o> PropertyHandlerContext<'i, 'o> {
}
}

pub fn child(&self, context: DeclarationContext) -> Self {
PropertyHandlerContext {
targets: self.targets,
is_important: false,
supports: Vec::new(),
ltr: Vec::new(),
rtl: Vec::new(),
context,
unused_symbols: self.unused_symbols
}
}

pub fn should_compile_logical(&self, feature: Feature) -> bool {
// Don't convert logical properties in style attributes because
// our fallbacks rely on extra rules to define --ltr and --rtl.
Expand Down
33 changes: 33 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9525,6 +9525,39 @@ mod tests {
..Browsers::default()
},
);

nesting_test_with_targets(
r#"
.foo {
padding-inline-start: 3px;

.bar {
padding-inline-start: 5px;
}
}
"#,
indoc! {r#"
.foo:not(:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi)) {
padding-left: 3px;
}

.foo:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi) {
padding-right: 3px;
}

.foo .bar:not(:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi)) {
padding-left: 5px;
}

.foo .bar:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi) {
padding-right: 5px;
}
"#},
Browsers {
safari: Some(12 << 16),
..Browsers::default()
}.into(),
);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/rules/keyframes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ impl<'i> KeyframesRule<'i> {
for keyframe in &mut self.keyframes {
keyframe
.declarations
.minify(context.handler, context.important_handler, context.handler_context)
.minify(context.handler, context.important_handler, &mut context.handler_context)
}

context.handler_context.context = DeclarationContext::None;
Expand Down
29 changes: 26 additions & 3 deletions src/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use self::font_palette_values::FontPaletteValuesRule;
use self::layer::{LayerBlockRule, LayerStatementRule};
use self::property::PropertyRule;
use crate::context::PropertyHandlerContext;
use crate::declaration::DeclarationHandler;
use crate::declaration::{DeclarationHandler, DeclarationBlock};
use crate::dependencies::{Dependency, ImportDependency};
use crate::error::{MinifyError, ParserError, PrinterError, PrinterErrorKind};
use crate::parser::{
Expand Down Expand Up @@ -466,7 +466,7 @@ pub(crate) struct MinifyContext<'a, 'i> {
pub targets: &'a Targets,
pub handler: &'a mut DeclarationHandler<'i>,
pub important_handler: &'a mut DeclarationHandler<'i>,
pub handler_context: &'a mut PropertyHandlerContext<'i, 'a>,
pub handler_context: PropertyHandlerContext<'i, 'a>,
pub unused_symbols: &'a HashSet<String>,
pub custom_media: Option<HashMap<CowArcStr<'i>, CustomMediaRule<'i>>>,
pub css_modules: bool,
Expand Down Expand Up @@ -668,6 +668,24 @@ impl<'i, T: Clone> CssRuleList<'i, T> {
.collect::<Vec<_>>();

context.handler_context.reset();

// If the rule has nested rules, and we have extra rules to insert such as for logical properties,
// we need to split the rule in two so we can insert the extra rules in between the declarations from
// the main rule and the nested rules.
let nested_rule = if !style.rules.0.is_empty() && (!logical.is_empty() || !supports.is_empty() || !incompatible_rules.is_empty()) {
let mut rules = CssRuleList(vec![]);
std::mem::swap(&mut style.rules, &mut rules);
Some(StyleRule {
selectors: style.selectors.clone(),
declarations: DeclarationBlock::default(),
rules,
vendor_prefix: style.vendor_prefix,
loc: style.loc
})
} else {
None
};

if !merged && !style.is_empty() {
let source_index = style.loc.source_index;
let has_no_rules = style.rules.0.is_empty();
Expand Down Expand Up @@ -714,6 +732,11 @@ impl<'i, T: Clone> CssRuleList<'i, T> {
}
rules.extend(supports);
}

if let Some(nested_rule) = nested_rule {
rules.push(CssRule::Style(nested_rule));
}

continue;
}
CssRule::CounterStyle(counter_style) => {
Expand Down Expand Up @@ -777,7 +800,7 @@ fn merge_style_rules<'i, T>(
.extend(style.declarations.important_declarations.drain(..));
last_style_rule
.declarations
.minify(context.handler, context.important_handler, context.handler_context);
.minify(context.handler, context.important_handler, &mut context.handler_context);
return true;
} else if style.declarations == last_style_rule.declarations
&& style.rules.0.is_empty()
Expand Down
5 changes: 4 additions & 1 deletion src/rules/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,14 @@ impl<'i, T: Clone> StyleRule<'i, T> {
context.handler_context.context = DeclarationContext::StyleRule;
self
.declarations
.minify(context.handler, context.important_handler, context.handler_context);
.minify(context.handler, context.important_handler, &mut context.handler_context);
context.handler_context.context = DeclarationContext::None;

if !self.rules.0.is_empty() {
let mut handler_context = context.handler_context.child(DeclarationContext::StyleRule);
std::mem::swap(&mut context.handler_context, &mut handler_context);
self.rules.minify(context, unused)?;
context.handler_context = handler_context;
if unused && self.rules.0.is_empty() {
return Ok(true);
}
Expand Down
4 changes: 2 additions & 2 deletions src/stylesheet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ where

/// Minify and transform the style sheet for the provided browser targets.
pub fn minify(&mut self, options: MinifyOptions) -> Result<(), Error<MinifyErrorKind>> {
let mut context = PropertyHandlerContext::new(options.targets, &options.unused_symbols);
let context = PropertyHandlerContext::new(options.targets, &options.unused_symbols);
let mut handler = DeclarationHandler::default();
let mut important_handler = DeclarationHandler::default();

Expand All @@ -212,7 +212,7 @@ where
targets: &options.targets,
handler: &mut handler,
important_handler: &mut important_handler,
handler_context: &mut context,
handler_context: context,
unused_symbols: &options.unused_symbols,
custom_media,
css_modules: self.options.css_modules.is_some(),
Expand Down

0 comments on commit e66d99c

Please sign in to comment.