Skip to content

Commit 64c8027

Browse files
jareddellittStranger6667
authored andcommitted
fix: preserve !important when merging styles onto elements with existing inline styles
Two bugs in `merge_styles` caused `!important` flags to be lost during CSS inlining when the target element already had an inline `style` attribute: 1. Separator mismatch with minify_css: The property lookup in the declarations buffer always used `": "` (STYLE_SEPARATOR), but when minify_css is enabled, buffered declarations use `":"` (STYLE_SEPARATOR_MIN). This caused the find to miss existing properties, leading to duplicated declarations and lost `!important` flags. 2. !important stripped unconditionally: The `strip_suffix("!important")` used internally to detect important rules was never re-appended to the output. Both the override path (Some, Some) and the new-property path (Some, None) wrote values without restoring the `!important` suffix. Bug 1 affects only `minify_css: true`. Bug 2 affects all configurations. This is particularly impactful for HTML email inlining where `keep_style_tags: true` is used alongside rules like `a[x-apple-data-detectors] { color: inherit !important }` that remain in `<style>` tags — the missing `!important` on inlined button styles allowed these reset rules to override intended button styling.
1 parent 6fa8127 commit 64c8027

File tree

2 files changed

+90
-10
lines changed

2 files changed

+90
-10
lines changed

css-inline/src/html/serializer.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,11 @@ fn merge_styles<Wr: Write>(
700700
let current_declarations_count = parsed_declarations_count;
701701
// Next, we iterate over the new styles and merge them into our existing set
702702
// New rules will not override old ones unless they are marked as `!important`
703+
let sep = if minify_css {
704+
STYLE_SEPARATOR_MIN
705+
} else {
706+
STYLE_SEPARATOR
707+
};
703708
for (property, _, value) in new_styles {
704709
match (
705710
value.trim_end().strip_suffix("!important"),
@@ -708,22 +713,22 @@ fn merge_styles<Wr: Write>(
708713
.take(parsed_declarations_count)
709714
.find(|style| {
710715
style.starts_with(property.as_bytes())
711-
&& style.get(property.len()..=property.len().saturating_add(1))
712-
== Some(STYLE_SEPARATOR)
716+
&& style
717+
.get(property.len()..property.len().saturating_add(sep.len()))
718+
== Some(sep)
713719
}),
714720
) {
715721
// The new rule is `!important` and there's an existing rule with the same name
716722
// Only override if the existing inline rule is NOT `!important`.
717723
// Per CSS spec: inline `!important` takes precedence over stylesheet `!important`
718724
(Some(value), Some(buffer)) => {
719725
if !buffer.ends_with(b"!important") {
720-
// We keep the rule name and the colon-space suffix - '<rule>: `
721-
buffer.truncate(property.len().saturating_add(STYLE_SEPARATOR.len()));
726+
buffer.truncate(property.len().saturating_add(sep.len()));
722727
write_declaration_value(buffer, value)?;
728+
buffer.extend_from_slice(b" !important");
723729
}
724730
}
725731
// There's no existing rule with the same name, but the new rule is `!important`
726-
// In this case, we add the new rule with the `!important` suffix removed
727732
(Some(value), None) => {
728733
push_or_update!(
729734
declarations_buffer,
@@ -732,6 +737,13 @@ fn merge_styles<Wr: Write>(
732737
value,
733738
minify_css
734739
);
740+
// `push_or_update!` increments `parsed_declarations_count`, so subtract 1
741+
// to get the slot that was just written and append the `!important` suffix.
742+
if let Some(buf) = declarations_buffer
743+
.get_mut(parsed_declarations_count.saturating_sub(1))
744+
{
745+
buf.extend_from_slice(b" !important");
746+
}
735747
}
736748
// There's no existing rule with the same name, and the new rule is not `!important`
737749
// In this case, we just add the new rule as-is

css-inline/tests/test_inlining.rs

Lines changed: 73 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ fn important() {
242242
assert_inlined!(
243243
style = "h1 { color: blue !important; }",
244244
body = r#"<h1 style="color: red;">Big Text</h1>"#,
245-
expected = r#"<h1 style="color: blue">Big Text</h1>"#
245+
expected = r#"<h1 style="color: blue !important">Big Text</h1>"#
246246
)
247247
}
248248

@@ -251,17 +251,17 @@ fn important_with_space_at_the_end() {
251251
assert_inlined!(
252252
style = "h1 { color: blue !important ; }",
253253
body = r#"<h1 style="color: red;">Big Text</h1>"#,
254-
expected = r#"<h1 style="color: blue">Big Text</h1>"#
254+
expected = r#"<h1 style="color: blue !important">Big Text</h1>"#
255255
)
256256
}
257257

258258
#[test]
259259
fn important_no_rule_exists() {
260-
// `!important` rules should override existing inline styles
260+
// `!important` rules should be added with the flag preserved
261261
assert_inlined!(
262262
style = "h1 { color: blue !important; }",
263263
body = r#"<h1 style="margin:0">Big Text</h1>"#,
264-
expected = r#"<h1 style="color: blue;margin: 0">Big Text</h1>"#
264+
expected = r#"<h1 style="color: blue !important;margin: 0">Big Text</h1>"#
265265
)
266266
}
267267

@@ -303,6 +303,74 @@ fn important_inline_wins_over_stylesheet_important() {
303303
);
304304
}
305305

306+
#[test]
307+
fn important_override_with_minify_css() {
308+
// With minify_css enabled, `!important` should still be preserved when overriding inline styles.
309+
// Regression: the property lookup used ": " (non-minified separator) even when styles were
310+
// stored with ":" (minified), causing duplicated properties and lost `!important` flags.
311+
let inliner = CSSInliner::options().minify_css(true).build();
312+
let html = r#"<html><head><style>h1 { color: blue !important; }</style></head><body><h1 style="color: red;">Big Text</h1></body></html>"#;
313+
let result = inliner.inline(html).unwrap();
314+
assert!(
315+
result.contains("style=\"color:blue !important\""),
316+
"Expected `color:blue !important` with minified separator. Got: {}",
317+
result
318+
);
319+
assert_eq!(
320+
result.matches("color").count(),
321+
1,
322+
"Property should not be duplicated. Got: {}",
323+
result
324+
);
325+
}
326+
327+
#[test]
328+
fn important_no_rule_exists_with_minify_css() {
329+
// With minify_css enabled, `!important` should be preserved when adding new rules
330+
let inliner = CSSInliner::options().minify_css(true).build();
331+
let html = r#"<html><head><style>h1 { color: blue !important; }</style></head><body><h1 style="margin:0">Big Text</h1></body></html>"#;
332+
let result = inliner.inline(html).unwrap();
333+
assert!(
334+
result.contains("color:blue !important"),
335+
"Expected `color:blue !important`. Got: {}",
336+
result
337+
);
338+
assert!(
339+
result.contains("margin:0"),
340+
"Existing margin should be preserved. Got: {}",
341+
result
342+
);
343+
}
344+
345+
#[test]
346+
fn important_multiple_properties_with_existing_inline() {
347+
// Multiple `!important` properties from a stylesheet should all be preserved when merging
348+
// with an element that has existing inline styles
349+
let inliner = CSSInliner::options().build();
350+
let html = r##"<html><head><style>.btn a { display: inline-block !important; padding: 12px 20px !important; text-decoration: none !important; }</style></head><body><div class="btn"><a href="#" style="display: block; color: #fff;">Click</a></div></body></html>"##;
351+
let result = inliner.inline(html).unwrap();
352+
assert!(
353+
result.contains("display: inline-block !important"),
354+
"display should have !important. Got: {}",
355+
result
356+
);
357+
assert!(
358+
result.contains("padding: 12px 20px !important"),
359+
"padding should have !important. Got: {}",
360+
result
361+
);
362+
assert!(
363+
result.contains("text-decoration: none !important"),
364+
"text-decoration should have !important. Got: {}",
365+
result
366+
);
367+
assert!(
368+
result.contains("color: #fff"),
369+
"Existing color should be preserved. Got: {}",
370+
result
371+
);
372+
}
373+
306374
#[test]
307375
fn font_family_quoted() {
308376
// When property value contains double quotes
@@ -332,7 +400,7 @@ fn font_family_quoted_with_inline_style_override() {
332400
style = r#"h1 { font-family: "Open Sans", sans-serif !important; }"#,
333401
body = r#"<h1 style="font-family: Helvetica; whitespace: nowrap">Hello world!</h1>"#,
334402
// Then it should be replaced with single quotes
335-
expected = r#"<h1 style="font-family: 'Open Sans', sans-serif;whitespace: nowrap">Hello world!</h1>"#
403+
expected = r#"<h1 style="font-family: 'Open Sans', sans-serif !important;whitespace: nowrap">Hello world!</h1>"#
336404
)
337405
}
338406

0 commit comments

Comments
 (0)