diff --git a/crates/oxc_angular_compiler/src/styles/encapsulation.rs b/crates/oxc_angular_compiler/src/styles/encapsulation.rs index 251abfaff..64fece20f 100644 --- a/crates/oxc_angular_compiler/src/styles/encapsulation.rs +++ b/crates/oxc_angular_compiler/src/styles/encapsulation.rs @@ -26,8 +26,60 @@ //! - `::ng-deep` → removed (deprecated but still supported) //! - Media queries, keyframes, etc. → preserved -/// Placeholder for comments during processing. -const COMMENT_PLACEHOLDER: &str = "%COMMENT%"; +use std::ops::Range; + +/// Comments are replaced by `%COMMENT%` placeholders during processing, +/// where `` is the comment's index in the extracted list. +/// +/// Angular uses a bare `%COMMENT%` here, but that only restores correctly when +/// every placeholder survives exactly once and in order. Neither holds: the +/// `:host-context()` pass duplicates selector text (and any placeholder in it) +/// once per generated permutation, and `polyfill-next-selector` discards text. +/// With an unindexed placeholder each duplicate consumes the *next* comment, +/// so a following `/*# sourceMappingURL=... */` gets teleported into the middle +/// of a selector and a literal `%COMMENT%` is left behind in the shipped CSS. +/// Carrying the index makes restoration independent of both count and order. +const COMMENT_PLACEHOLDER_PREFIX: &str = "%COMMENT"; + +/// The placeholder standing in for `comments[index]`. +fn comment_placeholder(index: usize) -> String { + format!("{COMMENT_PLACEHOLDER_PREFIX}{index}%") +} + +/// Find the comment placeholder at or after `from`, returning its byte range +/// and the comment index it carries. +fn find_comment_placeholder(s: &str, from: usize) -> Option<(Range, usize)> { + let mut at = from; + while let Some(rel) = s[at..].find(COMMENT_PLACEHOLDER_PREFIX) { + let start = at + rel; + let digits_at = start + COMMENT_PLACEHOLDER_PREFIX.len(); + let after = &s[digits_at..]; + let digits = after.bytes().take_while(u8::is_ascii_digit).count(); + if digits > 0 + && after.as_bytes().get(digits) == Some(&b'%') + && let Ok(index) = after[..digits].parse() + { + return Some((start..digits_at + digits + 1, index)); + } + at = digits_at; + } + None +} + +/// Remove every comment placeholder from `s`, returning the cleaned text and +/// the removed placeholders concatenated in their original order. +fn strip_comment_placeholders(s: &str) -> (String, String) { + let mut cleaned = String::with_capacity(s.len()); + let mut removed = String::new(); + let mut at = 0; + while let Some((range, _)) = find_comment_placeholder(s, at) { + cleaned.push_str(&s[at..range.start]); + at = range.end; + removed.push_str(&s[range]); + } + cleaned.push_str(&s[at..]); + (cleaned, removed) +} // Polyfill host markers (matching Angular's shadow_css.ts) const POLYFILL_HOST: &str = "-shadowcsshost"; @@ -354,6 +406,7 @@ fn extract_comments(css: &str) -> (String, Vec) { trimmed.starts_with('#') && trimmed[1..].trim_start().starts_with("source") }; + let index = comments.len(); if is_sourcemap { comments.push(comment.to_string()); } else { @@ -368,7 +421,7 @@ fn extract_comments(css: &str) -> (String, Vec) { comments.push(preserved); } - result.push_str(COMMENT_PLACEHOLDER); + result.push_str(&comment_placeholder(index)); } else { i += push_utf8_char(&mut result, css, i); } @@ -378,18 +431,26 @@ fn extract_comments(css: &str) -> (String, Vec) { } /// Restore comments from placeholders. +/// +/// Every placeholder carries the index of the comment it stands for, so a +/// placeholder that got duplicated (`:host-context()` permutations) restores to +/// the same comment as its twin, and one that got dropped shifts nothing. +/// +/// `extract_comments` only ever emits in-range indices, and the passes that +/// copy selector text copy the index along with it, so every placeholder *we* +/// generated resolves. An index that doesn't is therefore source CSS that +/// merely looks like a placeholder (`content: "%COMMENT7%"`), and is left +/// exactly as the author wrote it. fn restore_comments(css: &str, comments: &[String]) -> String { - let mut result = css.to_string(); - let mut idx = 0; + let mut result = String::with_capacity(css.len()); + let mut at = 0; - while result.find(COMMENT_PLACEHOLDER).is_some() { - if idx < comments.len() { - result = result.replacen(COMMENT_PLACEHOLDER, &comments[idx], 1); - idx += 1; - } else { - break; - } + while let Some((range, index)) = find_comment_placeholder(css, at) { + result.push_str(&css[at..range.start]); + result.push_str(comments.get(index).map_or(&css[range.start..range.end], String::as_str)); + at = range.end; } + result.push_str(&css[at..]); result } @@ -2048,6 +2109,30 @@ fn scope_selector_part_with_context( return String::new(); } + // Detach any comment placeholders before anything inspects the selector. + // A comment can be glued straight onto the selector text (PostCSS reprints + // `/* why */\n.foo` as `/* why */.foo`), and every check below is a + // whole-string match: with `%COMMENT0%` still attached, `:where(.one)` + // stops looking like a pure pseudo-function and gets scoped as + // `[content]:where(.one)` instead of `:where(.one[content])` - which is a + // real cascade change, since `:where()` contributes no specificity. The + // placeholders restore to blank text, so re-emitting them in front is + // enough. + if selector.contains(COMMENT_PLACEHOLDER_PREFIX) { + // `%COMMENT` without a valid index isn't ours - fall through and treat + // it as ordinary selector text. + let (stripped, placeholders) = strip_comment_placeholders(selector); + if !placeholders.is_empty() { + if stripped.trim().is_empty() { + // Nothing but placeholder(s) - no real selector to scope. Must + // not fall through, or a lone comment would become a bare + // `[content]` matching every element in the component. + return selector.to_string(); + } + return placeholders + &scope_selector_part_with_context(&stripped, ctx, part_has_host); + } + } + // If this part IS the host marker, don't add content attr if !ctx.host_marker.is_empty() && selector.trim() == ctx.host_marker { return selector.to_string(); @@ -2243,10 +2328,9 @@ fn scope_simple_selector(selector: &str, content_attr: &str) -> String { return String::new(); } - // Don't scope comment placeholders - if selector.contains(COMMENT_PLACEHOLDER) { - return selector.to_string(); - } + // Comment placeholders are detached in `scope_selector_part_with_context`, + // which is upstream of every caller of this function, so the selector text + // here is already placeholder-free. // Already has the content attribute let attr = format!("[{}]", content_attr); diff --git a/crates/oxc_angular_compiler/tests/shadow_css_test.rs b/crates/oxc_angular_compiler/tests/shadow_css_test.rs index 53b050d47..b88b71176 100644 --- a/crates/oxc_angular_compiler/tests/shadow_css_test.rs +++ b/crates/oxc_angular_compiler/tests/shadow_css_test.rs @@ -9,7 +9,7 @@ //! //! The goal is to have 1:1 compatibility with Angular's ShadowCss implementation. -use oxc_angular_compiler::styles::shim_css_text; +use oxc_angular_compiler::styles::{finalize_component_style, shim_css_text}; /// Normalize CSS for comparison (matches Angular's toEqualCss behavior). /// - Removes leading/trailing whitespace @@ -328,7 +328,6 @@ fn test_multibyte_utf8_preserved_in_css_values() { #[test] fn test_finalize_preserves_unicode() { - use oxc_angular_compiler::styles::finalize_component_style; // Full pipeline with Sass-compiled CSS containing actual bullet character let css = ".test:after { content: \"\u{2022}\"; }"; let result = finalize_component_style(css, true, "_ngcontent-%COMP%", "_nghost-%COMP%", true); @@ -1000,6 +999,298 @@ fn test_scope_first_selector_after_multiple_comments() { assert_css_eq!(shim(css, "contenta"), expected); } +#[test] +fn test_scope_selector_glued_directly_to_preceding_comment() { + // A comment with NO whitespace before the next selector. Nobody writes + // this by hand, but it's what PostCSS's AST reprint produces when a Vite + // consumer runs a `styleUrl` file through `preprocessCSS` (e.g. for + // Tailwind) before it reaches `shim_css_text`. Bailing out here shipped + // `.foo` with no content attribute at all - a global rule that clobbers + // any other component's `.foo`, with no console error. + assert_css_eq!( + shim("/* comment */.foo { color: red; }", "contenta"), + ".foo[contenta] { color: red; }" + ); +} + +#[test] +fn test_scope_selector_glued_directly_to_multiple_preceding_comments() { + assert_css_eq!( + shim("/* one *//* two */.foo { color: red; }", "contenta"), + ".foo[contenta] { color: red; }" + ); +} + +#[test] +fn test_scope_selector_glued_directly_to_trailing_comment() { + // Comment glued to the END of a selector, immediately before `{`. + assert_css_eq!( + shim(".foo/* comment */{ color: red; }", "contenta"), + ".foo[contenta]{ color: red; }" + ); +} + +#[test] +fn test_scope_selector_with_interior_glued_comment() { + // Placeholder *inside* the compound selector rather than at either end. + // These must scope too - an unscoped rule leaks globally either way. + assert_css_eq!( + shim(".foo/* c */:hover { color: red; }", "contenta"), + ".foo[contenta]:hover { color: red; }" + ); + assert_css_eq!( + shim(".foo/* c */.bar { color: red; }", "contenta"), + ".foo.bar[contenta] { color: red; }" + ); + assert_css_eq!( + shim("/* a */.foo/* b */.bar/* c */ { color: red; }", "contenta"), + ".foo.bar[contenta] { color: red; }" + ); +} + +#[test] +fn test_comment_only_selector_is_not_scoped() { + // A lone comment where a selector would go must stay a no-op - scoping it + // would emit a bare `[contenta]` matching every element in the component. + assert!(!shim("/* c */ { color: red; }", "contenta").contains("contenta")); + assert!(!shim("/* a */ /* b */ { color: red; }", "contenta").contains("contenta")); +} + +#[test] +fn test_scope_multiple_rules_after_comment_glued_selectors() { + // Several rules in one stylesheet, each with its comment glued on by + // PostCSS - all must scope, not just the first. + let css = "/* first */.foo { width: 1px; }/* second */.bar { height: 2px; }"; + let expected = ".foo[contenta] { width: 1px; }.bar[contenta] { height: 2px; }"; + assert_css_eq!(shim(css, "contenta"), expected); +} + +#[test] +fn test_scope_comment_glued_selectors_in_descendant_chain_and_comma_list() { + // The comment glues only to the *first* compound selector - every other + // part still needs its own attribute. + assert_css_eq!( + shim("/* c */.container .tabs-group { color: red; }", "contenta"), + ".container[contenta] .tabs-group[contenta] { color: red; }" + ); + assert_css_eq!( + shim("/* c */.a, .b { color: red; }", "contenta"), + ".a[contenta], .b[contenta] { color: red; }" + ); +} + +#[test] +fn test_scope_comment_glued_to_host_and_host_context() { + assert_css_eq!( + shim_with_host("/* c */:host { color: red; }", "contenta", "hosta"), + "[hosta] { color: red; }" + ); + assert_css_eq!( + shim_with_host("/* c */:host-context(.dark) { color: red; }", "contenta", "hosta"), + ".dark[hosta], .dark [hosta] { color: red; }" + ); + assert_css_eq!( + shim_with_host(":host/* c */.a { color: red; }", "contenta", "hosta"), + ".a[hosta] { color: red; }" + ); +} + +#[test] +fn test_comment_glued_keyframe_selectors_stay_unscoped() { + // `%COMMENT0%from` must still be recognised as a keyframe selector after + // the placeholders are stripped - scoping it would break the animation. + assert_css_eq!( + shim("@keyframes k {/* c */from { opacity: 0; }/* d */50% { opacity: 1; } }", "contenta"), + "@keyframes contenta_k {from { opacity: 0; }50% { opacity: 1; }}" + ); +} + +#[test] +fn test_comment_glued_selector_preserves_comment_newlines() { + // Comments restore as the newlines they contained, so the shimmed output + // keeps its line count and component sourcemaps don't shift. Stripping + // placeholders out of a selector must not drop any of them. Their exact + // position within the selector doesn't matter (they're blank text) - they + // come back out in front of it. + assert_eq!( + shim("/* multi\nline */.foo { color: red; }", "contenta"), + "\n.foo[contenta] { color: red; }" + ); + assert_eq!( + shim(".foo/* multi\nline */{ color: red; }", "contenta"), + "\n.foo[contenta]{ color: red; }" + ); + // An interior comment's newline must not land mid-compound-selector, where + // it would silently become a descendant combinator (`.foo :hover`). + assert_eq!( + shim(".foo/* multi\nline */:hover { color: red; }", "contenta"), + "\n.foo[contenta]:hover { color: red; }" + ); +} + +#[test] +fn test_comment_glued_stylesheet_stays_scoped_through_finalize() { + // End-to-end shape that surfaced this: a whole component stylesheet as + // PostCSS reprints it, with every documenting comment glued to the rule + // it documents. Run through the minifying entry point too, since that + // re-parses the shimmed output. + let css = "\ +:host { position: relative; }\ +/* Contributes width but no height. */\ +.probe { display: grid; height: 0; }\ +:host:has(.widget) { padding-left: 21px; }\ +/* This rule positions the widget precisely. */\ +.widget { width: 9px; position: absolute; }\ +.widget:hover { background-color: red; }\ +/* Sticky variant used when scrolling. */\ +.container.sticky-header { display: flex; }\ +"; + + for minify in [false, true] { + let result = + finalize_component_style(css, true, "_ngcontent-%COMP%", "_nghost-%COMP%", minify); + assert!(!result.contains("%COMMENT"), "placeholder leaked (minify={minify}):\n{result}"); + for selector in [".probe", ".widget", ".container.sticky-header"] { + assert!( + result.contains(&format!("{selector}[_ngcontent-%COMP%]")), + "`{selector}` lost its content attribute (minify={minify}):\n{result}" + ); + } + } +} + +// ============================================================================ +// Regression: comment placeholders must never survive into the shipped CSS +// ============================================================================ + +/// Inputs that exercise every pass which duplicates, drops or reorders the +/// selector text a comment placeholder can be embedded in. +const PLACEHOLDER_STRESS_CASES: &[&str] = &[ + // `:host-context()` emits its trailing selector text once per permutation, + // duplicating any placeholder inside it. + ":host-context(.d) /* c */ .e { color: red; }", + ":host-context(.d)/* c */ .e { color: red; }", + ":host-context(.d) .e/* c */ { color: red; }", + ":host-context(/* c */.d) .e { color: red; }", + ":host-context(.a, .b) /* c */ .e { color: red; }", + ":host-context(.a):host-context(.b) /* c */ .e { color: red; }", + ":where(:host-context(.d)) /* c */ .e { color: red; }", + "/* pre */:host-context(.d) /* c */ .e { color: red; }", + ":host-context(.a) /* c */ :host { color: red; }", + // Passes that discard selector text, dropping placeholders. + "polyfill-next-selector { content: ':host .a'; }/* c */::content .b { color: red; }", + "polyfill-rule { content: ':host .a'; color: red; }/* c */.z { color: blue; }", + "polyfill-unscoped-rule { content: '.a'; color: red; }/* c */.z { color: blue; }", + ".a ::ng-deep /* c */ .b { color: red; }", + // Ordinary shapes, glued and unglued. + "/* c */.foo/* d */.bar/* e */ { color: red; }", + "@media screen { /* c */.foo { color: red; } }", + "@keyframes k {/* c */from { opacity: 0; } }", + "/* c */ { color: red; }", +]; + +#[test] +fn test_comment_placeholder_never_leaks_into_output() { + // A literal `%COMMENT%` in shipped CSS is a corrupt selector. Before + // placeholders carried their comment index, every `:host-context()` case + // below leaked one: each duplicated placeholder consumed the *next* + // comment, so the last one ran out and was left in the output verbatim. + for css in PLACEHOLDER_STRESS_CASES { + let result = shim_with_host(css, "contenta", "hosta"); + assert!(!result.contains("%COMMENT"), "placeholder leaked for {css:?}:\n{result}"); + } +} + +#[test] +fn test_sourcemap_comment_survives_placeholder_duplication() { + // Sourcemap comments are the one kind restored verbatim, so a mis-mapped + // placeholder is directly visible: before the fix the duplicated + // `:host-context()` placeholder consumed this comment and teleported it + // into the middle of a selector, leaving `%COMMENT%` at the real position. + for css in PLACEHOLDER_STRESS_CASES { + let with_map = format!("{css}\n/*# sourceMappingURL=x.map */"); + let result = shim_with_host(&with_map, "contenta", "hosta"); + // It must still occupy its own line - the teleport spliced it into the + // middle of a selector. (`polyfill-unscoped-rule` legitimately appends + // its rule after this line, so don't require it to be last.) + assert!( + result.lines().any(|line| line.trim() == "/*# sourceMappingURL=x.map */"), + "sourcemap comment moved for {css:?}:\n{result}" + ); + assert_eq!( + result.matches("sourceMappingURL").count(), + 1, + "sourcemap comment duplicated for {css:?}:\n{result}" + ); + } +} + +#[test] +fn test_placeholder_shaped_text_in_source_css_is_left_alone() { + // Author-written text that merely looks like a placeholder must survive. + // Every placeholder we generate resolves (`extract_comments` only emits + // in-range indices and the duplicating passes copy the index with the + // text), so an index that doesn't resolve is the author's, not ours. + for css in [ + ".label { content: \"%COMMENT0%\"; }", + ".label { content: \"%COMMENT99%\"; }", + "/* c */.label { content: \"%COMMENT7%\"; }", + ] { + let result = shim(css, "contenta"); + assert!(result.contains("content: \"%COMMENT"), "declaration text was eaten: {result}"); + assert!(result.contains(".label[contenta]"), "{result}"); + } +} + +#[test] +fn test_comment_glued_to_pure_pseudo_function_scopes_inside_it() { + // `:where()` contributes no specificity, so scoping to `[contenta]:where(.one)` + // instead of `:where(.one[contenta])` silently changes the cascade. A glued + // comment must not make the part stop looking like a pure pseudo-function. + assert_css_eq!( + shim("/* c */:where(.one) { color: red; }", "contenta"), + ":where(.one[contenta]) { color: red; }" + ); + assert_css_eq!( + shim(":where(.one)/* c */ { color: red; }", "contenta"), + ":where(.one[contenta]) { color: red; }" + ); + assert_css_eq!( + shim("/* c */:is(.a, .b) { color: red; }", "contenta"), + ":is(.a[contenta], .b[contenta]) { color: red; }" + ); + assert_css_eq!( + shim("/* c */:where(.a):is(.b) { color: red; }", "contenta"), + ":where(.a[contenta]):is(.b[contenta]) { color: red; }" + ); +} + +#[test] +fn test_host_context_permutations_share_one_comment() { + // Every permutation gets a copy of the placeholder; each must restore to + // the same (blank) comment rather than eating a later one. + assert_css_eq!( + shim_with_host(":host-context(.a, .b) /* c */ .e { color: red; }", "contenta", "hosta"), + ".a[hosta] .e[contenta], .a [hosta] .e[contenta], \ + .b[hosta] .e[contenta], .b [hosta] .e[contenta] { color: red; }" + ); + assert_css_eq!( + shim_with_host(":host-context(/* c */.d) .e { color: red; }", "contenta", "hosta"), + ".d[hosta] .e[contenta], .d [hosta] .e[contenta] { color: red; }" + ); +} + +#[test] +fn test_comment_placeholder_indices_are_independent_of_position() { + // Restoration is by index, so a dropped placeholder earlier in the file + // must not shift the comments that follow it onto the wrong placeholders. + let css = "polyfill-next-selector { content: ':host .a'; }/* dropped */::content .b { color: red; }\n\ + /*# sourceMappingURL=x.map */"; + let result = shim_with_host(css, "contenta", "hosta"); + assert!(result.contains("/*# sourceMappingURL=x.map */"), "{result}"); + assert!(!result.contains("%COMMENT"), "{result}"); +} + #[test] fn test_newline_as_descendant_combinator() { // Newline between selectors is a valid CSS descendant combinator