Repository navigation
Conversation
…eeds A fallback declaration for a vendor prefix rewrote every layer of a background image list when any layer needed that prefix. A gradient beside image-set() became -webkit-radial-gradient(circle at 0 0, ...), which the prefixed grammar rejects, so the declaration was invalid in the browsers it was made for. get_fallbacks_image now decides per layer: - A prefixed declaration exists only when an unprefixed layer needs the prefix. In it, an unprefixed layer takes the prefix only if it needs it. - A gradient that the author prefixed still takes the prefix of a generated declaration. image-set() never becomes -moz- or -o-image-set(). - A list in which every layer has an author prefix stays as written, with RGB colors. Gradient::get_legacy_webkit returns a -webkit-gradient() as it is, so the legacy declaration keeps that layer. VendorPrefix::from_name_str has no caller left and is removed.
|
Updated 7:50 AM PT - Oct 25th, 2026
✅ @robobun, your commit a875325ca886df62891d1486efeb50b9f38a7c22 passed in 🧪 To try this PR locally: bunx bun-pr 46755That installs a local version of the PR into your bun-46755 --bun |
|
Status: closed, not merged. #47025 consolidated the open CSS parser pull requests and did not take this change (see the comment of Jarred-Sumner below). Reopen this PR if the input shows up in real CSS. The reproduction, for the record (bun 1.4.3, bun 1.4.4 and main 5c6d4e2, default browser targets): printf 'a{background-image:radial-gradient(circle at top left,red,blue),image-set("data:image/png;base64,AA==" 1x)}' > a.css
bun build a.css --outdir=o && cat o/a.cssThe first declaration of the output is |
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked the other callers of small_list::get_fallbacks for the same per-layer prefix change: only the two BackgroundHandler::flush sites in src/css/properties/background.rs use it, and border-image goes through the single-image Image::get_fallbacks path, so it is unaffected. The deleted VendorPrefix::from_name_str has no remaining callers in src/css.
Extended reasoning...
The change rewrites vendor-prefix fallback generation for background and background-image layer lists in src/css/small_list.rs, adds an early return in Gradient::get_legacy_webkit, deletes an unused helper in src/css/lib.rs, and adds a block of prefix tests in test/js/bun/css/css.test.ts; it touches no security-sensitive surface. Two confirmed findings are posted inline, so the inline comments already signal that a human look is needed; this note only records the sibling call sites and dead-code deletion that were checked and ruled out.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/css/small_list.rs— Users targeting legacy WebKit (chrome 8, safari 5) still get the samebackground-imagedeclaration printed twice for a list such as-webkit-image-set(url(a.png) 1x), url(b.png). The legacy block at src/css/small_list.rs:116 is gated on the unionprefixes, which an author-prefixed image-set satisfies, and src/css/values/image.rs:139 deep-clones every non-gradient layer, so an identical list is pushed at src/css/small_list.rs:130. The PR replaced the union gate for prefix_helper withunprefixed_needsbut left this one. Fix: push the legacy list only when at least one layer was actually converted by get_legacy_webkit, or gate it onunprefixed_needsplus author-prefixed gradients, covering every list with no convertible gradient. [also at: src/css/values/gradient.rs:185 - Users building for legacy WebKit targets (Chrome 4-9, iOS Safari 3.2-6) now get the same background-image declaration printed twice when a list mixes an author-webkit-gradient()with layers that have no legacy form. src/css/values/gradient.rs:183 now returns the-webkit-gradient()unchanged, and src/css/small_list.rs:123 deep-clones url()/image-set layers, so the legacy list at src/css/small_list.rs:130 can equal the written value with no dedup downstream.]Why this was flagged
Input: a background or background-image list where no layer is a standard gradient but one layer carries
-webkit-(e.g.-webkit-image-set(url(a.png) 1x), url(b.png)), under browser targets for which Feature::is_webkit_gradient is true (chrome < 10, safari <= 5.0, ios_saf <= 4). At src/css/small_list.rs:91-92 the author image-set contributes WEBKIT toprefixes(targets.rs:128 returns the author prefix unchanged), so the gate at :116 passes. The loop at :122-126 calls Image::get_legacy_webkit, which at src/css/values/image.rs:139 returns a deep clone for every non-gradient layer, soimagesequals the original list and :130 pushes it. The caller prints it, then prints the original: the same declaration twice. The PR's own goal is to stop emitting redundant relabelled copies, and it introducedunprefixed_needsat :87 for prefix_helper, but the legacy gate still uses the union. Base also emitted this duplicate (plus a third copy from prefix_helper). Remedy: track whether any get_legacy_webkit call changed a layer and only push when it did.Verification: Pre-existing: the base emits it three times; the PR cuts it to two but leaves the legacy-webkit path on the union gate. Trigger: an author
-webkit-image-set()beside an unprefixed non-gradient layer under targets whereFeature::is_webkit_gradientholds. src/css/small_list.rs:116 gates on the unionprefixes, so image.rs:139 deep-clones every layer and line 130 pushes an identical copy.
`prefix_helper` decides per layer and pushes its list only when a layer changed. An unprefixed gradient takes the prefix only if it needs it. A gradient with an author prefix takes it too, so a list in two author prefixes keeps one declaration per prefix. In a `-webkit-` declaration every `image-set()` is `-webkit-image-set()`, also when the targets give `image-set()` no need of its own. The `-webkit-gradient()` pass and `Gradient::get_legacy_webkit` go back to their form on main.
|
The review has two findings. 35a3c7a handles the first one. This PR does not change the second one.
|
There was a problem hiding this comment.
I reviewed the latest push (35a3c7a) and found no bugs; it resolves the mixed-author-prefix case I flagged earlier, and the new test row pins it. Because this deliberately diverges from the lightningcss union behavior on a couple of judgment calls (an unprefixed image-set() always takes -webkit- inside a webkit declaration; a two-vendor author list is replaced by the last generated one), a human look at those choices is still worthwhile.
What was reviewed:
- Traced
prefix_helperand the trailingfirst_prefixed/rgbbranches againstget_necessary_prefixes,get_vendor_prefix,get_prefixed, andTargets::prefixesfor same-prefix, mixed-prefix, legacy-webkit-gradient()-present, and rgb-present/absent lists; each matches the old output or the new test expectations. - Confirmed
VendorPrefix::from_name_strhas no remaining callers andhas_vendor_prefixis still used byBackgroundHandler. - Note the description is now stale in two places:
src/css/values/gradient.rsis unchanged from base at HEAD, so an author-webkit-gradient()layer is still dropped from the legacy declaration (pre-existing, not a regression), and the two-author-prefix row now yields one declaration per vendor rather than "as written".
Extended reasoning...
The diff touches the CSS vendor-prefix fallback generator for background image layer lists (src/css/small_list.rs), deletes an unused panicking helper in src/css/lib.rs, and adds ~400 lines of tests including a pairwise layer matrix. It touches no security-sensitive surface. The second commit addressed the one inline finding from the prior run, and tracing every branch combination against the helper implementations found no behavioral regression. Deferring rather than approving because the change intentionally departs from the lightningcss reference on design points a maintainer should weigh, it is stacked on another open PR, and the PR description no longer matches the code in two places.
|
No code change follows from this review. Two notes for the reader. Description. I updated the title and the description for 35a3c7a after the push. They now say these two things:
Deviations from lightningcss. The two behaviors that the review names are not deviations. Main and lightningcss do the same.
The Downsides section names the one kind of output that a browser older than the targets loses. |
|
A note for the merge order with #46870. #46870 stops typing a prefixed radial gradient that has Two rows that this PR adds use such a value as typed input, both at
They need new expectations in the PR that lands second. Git merges |
Two rows used an author-prefixed radial gradient with a bare length as input. #46870 changes how such a value is parsed. A linear gradient takes the same path through the list routine, so the rows now hold in either merge order.
There was a problem hiding this comment.
I re-reviewed the latest push and found no bugs; the mixed-author-prefix case I raised earlier is now handled (an author-prefixed gradient is relabelled in every generated declaration, with a test row for the -webkit-/-moz- pair). Because this deliberately diverges from lightningcss's per-list prefixing and drops the all--webkit- copy that Chrome 10-25 relied on, a maintainer's call on that tradeoff would still be worthwhile.
What was reviewed:
- Traced
prefix_helperand thefirst_prefixed/rgbtail branches in src/css/small_list.rs against the old control flow for single-layer, legacy-webkit-gradient(), RGB-fallback-without-unprefixed-layer, and two-vendor lists — output matches the old code where no layer's prefix changes, and theres.pop()can no longer consume the legacy list. - Checked the retargeted
defaultTargets->chrome25rows: the direction property they pinned is still asserted, and the new default-target behavior is asserted alongside. - Confirmed
VendorPrefix::from_name_strhas no remaining callers and thatGradient::WebkitGradient(need = NONE) is never relabelled by the new gradient arm.
Extended reasoning...
The diff touches src/css/small_list.rs (per-layer vendor-prefix decision in get_fallbacks_image), removes an unused panicking helper from src/css/lib.rs, and adds ~400 lines of tests in test/js/bun/css/css.test.ts; no security-sensitive surface is involved. The code is small and I traced each branch against the previous behavior without finding a regression, and the prior inline concern about two-vendor lists was addressed in commit 35a3c7a. The decision to defer rather than approve rests on the change being a deliberate output-semantics tradeoff (dropping the all-webkit copy for pre-26 Chrome, diverging from lightningcss) that a CSS maintainer should weigh, not on any defect found. No CODEOWNERS entry covers these paths and no third-party objections are outstanding.
|
No code change follows from this review. The tradeoff that it names is in the description, under Downsides: a list such as The last push (a875325) changes tests only. Two rows at Chrome 8 had an author-prefixed radial gradient with a bare length as input. #46870 changes how such a value is parsed. The rows now use linear gradients, which take the same path through the list routine. On a tree that has this PR and #46870, |
### What does this PR do? Consolidates the open pull requests on the CSS parser (`src/css`: parsing, minifying, lowering, printing, CSS modules) into one. 179 open pull requests were in scope. Each was judged by one rule: **the output must mean to a browser what the input meant.** Every claim was run through a build of `main`, a build with the change, lightningcss 1.33 and Chromium 156 (CSSOM, computed values, painted pixels for colors and gradients), and counted against 10,116 real stylesheets: the published CSS of about 190 popular npm packages, plus stylesheets from open source repositories. 107 land here, whole or in part, most of them cut down. The source diff is net negative. Of the 10,116, 9,509 are stylesheets that someone ships. The rest are test fixtures of other tools (521), Tailwind v4 source that needs its plugin (59) and SCSS, PostCSS or template source (27). | 9,509 shipped stylesheets | `main` | this branch | |---|---|---| | fail to build | 71 | 35 | | ... of those, valid CSS that Bun rejects | | 0 | | minified output reads differently in Chromium than the input | 552 | 2 | | CSS modules with a name that is referenced but not defined, or exported but not defined | 166 | 0 | | all 10,116 | `main` | this branch | |---|---|---| | fail to build | 192 | 141 | | minified output reads differently in Chromium than the input | 593 | 15 | | build on `main`, fail here | | 0 | | right on `main`, wrong here (minified, and at the default targets) | | 0 | | size of the minified output, no targets | 162.32 MB | 162.47 MB (+0.093%) | | size of the output at the default targets | 224.1 MB | 210.2 MB (-6.2%) | A syntax error is a build error, with one exception: the IE star hack `*zoom: 1` is skipped, because tachyons and handsontable ship it. The 35 shipped stylesheets that fail are invalid CSS: 28 have a selector that Chromium drops (semantic-ui-css, swiper, @ionic/core, Adobe Spectrum, xp.css), 2 an `@import` after other rules, and one each `@supports not X and Y`, a nested `@keyframes`, a stray comma, a selector list in `::slotted()`, `@media screen\0` (govuk-frontend). The 2 that read differently fold a `color-mix()` inside a gradient (see the known gaps). The growth of the minified output is rules and declarations that `main` deleted by mistake: 87 KB of rules, 45 KB of prefixed declarations that only browser targets can rule out. #### Popular CSS that `main` breaks today - normalize.css, modern-normalize, sanitize.css: `font-family: monospace, monospace` is collapsed to `monospace`, which changes the font size from 16px to 13px (#46662). - CKEditor 5, Tabler, Carbon, VitePress, Ionic, PatternFly, Excalidraw: a rule of custom properties is deleted as a "duplicate" of a later rule with the same selector and other custom properties. 37 stylesheets lose 1,681 custom properties (#33684). - Material Components, daisyUI, DocSearch, plyr: a `var()` size loses to its own fallback (#42101). - nextra, daisyUI, CKEditor: declarations written after a nested rule are moved above it (#45462). - antd 4, purecss, govuk-frontend: browser hacks in selector lists start to apply everywhere (#42100). - foundation-sites, flatpickr, 98.css, blueprintjs, scalar, react-datepicker: the build fails on valid CSS (#46756, #38552, #38534). tachyons, handsontable: it fails on the IE star hack (#46751). - nodejs.org, Mantine: a CSS module exports classes that no rule defines, because the `@apply` or `@mixin` in them is moved out of the rule (#46759). - hint.css and 42 more: `z-index: 1000001` and `1000000` become the same number (#46715). - Adobe Spectrum: `--version: 5.0.1` prints as `5.1`. - A quarter of all stylesheets: minified output has a stray space in `align-items: center` (#41993). #### Behavior changes to look at first 1. **`light-dark()` is kept as written at the default browser targets** (#42009). The lowered form `var(--buncss-light, a) var(--buncss-dark, b)` is invalid unless a compiled `color-scheme` covers the element: 184 of the 215 real stylesheets that get it never define the variable, so every browser drops the declaration. The docs section on the polyfill is removed. 2. **`border` is not built from longhands** (#42072): `border-width:0;border-style:solid;border-color:red` stays three declarations, because `border` also resets `border-image`. 305 real stylesheets grow by 30 KB in total; `bootstrap.min.css` by 113 bytes, 5 gzipped. No real stylesheet was found where the old merge shows, so this is the one to drop if size matters more. 14 existing expectations change. 3. **A CSS module no longer exports names that only occur inside `:global(...)`** (#45487). 73 real modules lose 202 export keys, all of them names that no rule has. `import { g }` or `composes: g` of such a name becomes a build error. 4. **Container names and `@counter-style` names stay global in a CSS module** (#45247 by @mikamikasuki). `main` hashes the `@container` name but not `container-name`, so a named query never matches. postcss-modules (Vite, css-loader, Next.js) keeps them global; lightningcss and esbuild hash both. Global keeps a container that is queried from another file or from JS working. 5. **Syntax errors are build errors, also two that `main` lets through**: a media query that stops short (`@media screen and {`, `@media not {`, `@import "a.css" screen and;`) and `@media (width <) foo {`. No stylesheet of the 10,116 has either. A condition that is valid by the grammar though no browser knows it, `( ... )` or `fn( ... )`, is kept as written (#46756): `(min-width:0\0)`, `(not(hover))`. 6. **Declarations after a nested rule stay where they were written**, as a nested declarations rule (#45462). Changes the output of 199 real stylesheets; in a fuzz of 2,850 nested programs compared by computed style, `main` was wrong in 1,056 and the pull request's code, before it was trimmed here, in none. 7. **`Bun.color`**: `color()` inputs are converted instead of returned as a string (`Bun.color("color(srgb 1 0 0)", "hex")` was `"color(srgb 1 0 0)"`); `currentColor`, system colors and `light-dark()` give `null` in the converting formats instead of the keyword as a string; a relative color or `color-mix()` outside sRGB is clipped, not gamut-mapped (`hsl(from oklch(100% .399 336.3) h s l)` is `#ff5bff`, was `#fff`). 8. The `:-moz-any()` and `:-webkit-any()` copies of rules with `:is()` are gone at the default targets (#45945, #42029). The `:-webkit-any()` copy outranked the original rule: `.title{color:blue} :is(h1,h2,h3){color:red}` painted red. 5.8% of all default-target output was the `:-moz-any()` copies. 9. `Property` grows from 216 to 224 bytes: `Url` stores its text as a slice so that two `url()` values can be compared (#46895). The pull request packed a pointer and a `u32` behind `unsafe` to avoid that. #### Not in this PR, needs a decision - **Raise the default browser targets (#40369) and add a `cssTarget` option (#40368).** Today: Chrome 80, Edge 80, Firefox 78, Safari 14, Opera 67, and no public way to change them. At Vite 8's floor the output of the 9,924 stylesheets is 15.3% smaller (3.9% gzipped). About a dozen fixes here only matter at today's floor, and code for older targets is unreachable without the option. - Vendor prefixes for `user-select`, `appearance`, `hyphens` and five more (#45814): 1,116 real stylesheets lack a prefix they need at the default targets. And for `backdrop-filter` (#44648): 173. - Clip instead of gamut-map in the sRGB fallback of a plain wide-gamut color: deletes about 90 lines, changes every pinned fallback hex. If the mapping stays, #44647 by @oddharsh makes it closer to what browsers paint (colors off by 2/255 or more: 168 to 110 of 3,170 real ones). #### Changes **Rules: duplicates, merging, nesting, fallbacks** - The `:-moz-any()` copy of every rule with `:is()` or `:not(a, b)` is gone: Firefox 78 has `:is()` (#45945). 1,164 real stylesheets - Keep declarations written after a nested rule in place with a nested declarations rule (#45462). 379 real stylesheets - With nesting compiled away, an unknown at-rule inside a style rule was moved out of it: `.wrapper{@apply flex}` printed a bare `@apply flex;` and the rule was gone, so a CSS module exported a class that no rule defines. It now stays in a block under the compiled selector. 72 real modules with Tailwind's `@apply`, 25 with Mantine's `@mixin` (#46759). 316 real stylesheets - Keep !important in the rules generated for logical properties and color-scheme (#38597). 311 real stylesheets - With browser targets, remove an earlier duplicate rule only when its declaration block is equal (#46861). 171 real stylesheets - Compare custom property names in `PropertyId` equality and importance in duplicate-rule detection (#33684). 40 real stylesheets - Keep same-condition @supports rules and empty named @layer blocks (#46733). 31 real stylesheets - Keep target-added vendor prefix passes on cloned and re-minified rules, and never print a bare `:full-screen` (#43339). 12 real stylesheets - Keep the selectors on rules added for color fallbacks and logical properties (#43240). 3 real stylesheets - A later declaration of a property drops the `@supports` color copies of the earlier one, which printed after the rule and won (#45560). 2 real stylesheets - Do not merge style rules in a stylesheet that holds `revert-rule` (#46953). 1 real stylesheets - `@import ... supports(-webkit-mask: none)` kept its prefix (#38589) **Properties and shorthands** - Print align-items:center without a stray space when no safe/unsafe keyword is given (#41993). 3,317 real stylesheets - Emit the physical longhands instead of the inset shorthand for targets without it (#38583). 1,452 real stylesheets - Keep the stops of an upward gradient as written, and drop a `50%` hint only at the midpoint (#46790). 104 real stylesheets - Emit unparsed border-block-* declarations when compiling logical borders (#38582). 89 real stylesheets - Keep repeated font families and dedupe only the inserted system-ui fallbacks (#46662). 88 real stylesheets - A `width` / `height` / `min-*` / `max-*` with `var()` or `env()` was printed before the plain fallback written above it, so the fallback won (#42101). 26 real stylesheets - Keep box-orient and box-direction when flex-direction or flex-flow follows (#45672). 16 real stylesheets - Write held prefixed background declarations at every flush (#46882). 6 real stylesheets - Derive equality for Size2D and BorderRadius, and keep every prefix in the border-radius handler (#45665). 2 real stylesheets - Print `none` instead of an empty border-image, mask-border or mask-box-image value (#45143). 2 real stylesheets - Keep font/transition sub-properties on the side of the shorthand they were written on (#41990). 2 real stylesheets - CSS-wide keywords are not names: `font:12px Arial;font-family:inherit` printed `font:12px inherit`, `color-scheme:inherit` and `color-scheme:auto` printed `normal`, `transition-property:unset` was folded into `transition` (#42022). 2 real stylesheets - Only fold font longhands into `font` when the rule declared a `font` (#45459) - Keep a font family name quoted when its identifiers do not read back as the same name (#46606) - Stop rewriting valid positions into ones browsers reject (#45197) - `border` is no longer built from longhands unless the rule also sets what `border` resets (`border-image`), and an author's `border-image` reset is no longer dropped. `transition` is likewise only written where the rule wrote one, because it resets `transition-behavior` (#42072) - `border; border-image: <gradient>; border-image-slice: 1` lost the gradient in every default build (#42018) - A prefixed `linear-gradient` keeps its direction, written or generated: the prefixed syntax names the starting side and measures angles differently (#46697) **At-rules** - Print @font-face format() keywords as strings (#37979). 439 real stylesheets - Print @supports `not` conditions without a leading space (#38559). 88 real stylesheets - Reset after the number token of a unicode-range (#45835). 33 real stylesheets - Keep `<general-enclosed>` media and container conditions as written (#46756). 24 real stylesheets - A strict range such as `(width < 768px)` lowered for old targets still matched at exactly 768px. The step is now `.02px`. The `not all and` form for other units is not taken (#46806). 18 real stylesheets - Keep scanning an @supports condition after a nested block (#38552). 9 real stylesheets - Every `@counter-style name` printed as `@counter-stylename`, which browsers drop (also #46692). In a CSS module the name now stays global, like the `list-style` that refers to it (#33679). 9 real stylesheets - Keep a keyframe whose block has an invalid item (#46288). 2 real stylesheets - Read an @Property initial-value on the stylesheet's parser, keep an unmatched value as written (#45806). 2 real stylesheets - `@media {}` is `all`. A media query that stops after `and`, `or` or `not` (`@media screen and {`) is a build error; `main` printed `not all` or dropped the condition, silently. The `@layer` and type selector parts are not taken (#46869). 2 real stylesheets - Accept a name-only `@container` and a `style()` feature with no value (#46689). 1 real stylesheets - Parse `<timeline-range-name> <percentage>` keyframe selectors (#45652) - Allow `or` inside parentheses after a media type (#45088) - Print the `=` comparison in media and container queries as `=` (#45071) - Parse the @font-palette-values font-family descriptor as a list of family names (#45484) **Selectors** - Never wrap a selector list in :is() (#46787). 414 real stylesheets - Stop emitting :-webkit-any() copies of :is() that outrank the original rule (#42029). 373 real stylesheets - With browser targets, a selector list with a vendor-prefixed member was split, which revives hacks that rely on other engines dropping the whole list: `.s *::-ms-backdrop,.s{flex:auto}` applied everywhere. Rewritten; the list is printed as written (#42100). 42 real stylesheets - Print the vendor prefix on downleveled :any-link, :read-only, :read-write, :placeholder-shown and :autofill (#42131). 12 real stylesheets - Fix nesting lowering of a `&` that does not lead its selector (#43047). 8 real stylesheets - `@scope`: `&` and implicit selectors compile to `:where(:scope)`, relative selectors and bare declarations parse (#43165). 7 real stylesheets - Only reject bare :global / :local pseudo-classes in CSS modules (#38534). 5 real stylesheets - Stop truncating `@scope to (...)` and stop replacing `&` in the scope-end selector (#43146). 2 real stylesheets - `[xl|href$=".pdf"]` lost its namespace prefix (#45159). 2 real stylesheets - Match An+B keywords exactly and take only `+` as the leading sign (#42953). 1 real stylesheets - Parse ::details-content and ::picker() like ::part() (#45504). 1 real stylesheets - Parse :window-inactive as an ordinary pseudo-class, and accept it after ::selection (#45815) - Print :nth-col(1) and :nth-last-col(1) in functional form (#41575) - `li::before::marker` failed the build (#45810) - `&` inside `:nth-child(... of S)` is resolved when nesting is compiled away. The second selector serializer (326 lines) is deleted (#43024) **Colors and `Bun.color`** - Keep light-dark() as written under the default browser targets (#42009). 227 real stylesheets - Number and percentage channels parse in `lab()`, `lch()`, `oklab()`, `oklch()`, `hsl()` and `hwb()`; the hue of `lch` / `oklch` is no longer scaled by alpha (#45084). 130 real stylesheets - `Bun.color` returns `null` for `currentColor`, system colors and `light-dark()` in the converting formats, instead of the CSS keyword as a string (#33047). 130 real stylesheets - Relative colors: channel keywords are on the scale of their function (`rgb(from #369 calc(r + 10) g b)`), the origin's alpha is kept, a relative color kept as written gets no `@supports` copies, and a result outside sRGB is not folded (#45362). 31 real stylesheets - Keep rotate: 0deg out of none, and keep the origin alpha in relative colors (#38486). 13 real stylesheets - Compute the D50 reference white in f32 so greys convert to lab(l 0 0) like lightningcss (#38535). 7 real stylesheets - Return a missing hue for a grey from conversions into lch, oklch, hsl and hwb (#45201). 5 real stylesheets - Fix color(a98-rgb), srgb-linear relative colors and color-mix(in xyz-d50) (#38487). 4 real stylesheets - Match named colors without regard to ASCII case (#45092). 3 real stylesheets - Fold min(), max() and clamp() over plain numbers (#39515) - `calc(l - .08)` in a percentage channel of a relative color became NaN and printed `#000` (also #45970) (#45971) - Bun.color: convert color() inputs instead of returning them as a CSS string (#38488) - A `color-mix(in srgb, ...)` result outside sRGB prints `color(srgb ...)` instead of a gamut-mapped hex (#39269) **`calc()` and numbers** - Rounding to 6 significant digits ate into the integer part: `z-index:2147483647` became `2147480000`, `max-width:9999999px` became `10000000px`, `--x:1234567.5` became `1234570`. It now only cuts a fraction short: a value kept as tokens prints its source text, a typed value prints every integer digit and at least one digit of the fraction (#46715). 43 real stylesheets - A number without a unit is no longer repaired into `px` (`height:1.5715` is dropped by a standards-mode browser, so it prints as written), `calc(-1 * 0)` is not a length, and on a property of the range [0,∞] (sizes, border radii, padding, border and outline widths, gap, `flex-basis`, `font-size`) a `calc()` that folds below zero prints `0` instead of an invalid negative literal, and a negative literal no longer replaces the valid declaration before it (`width:10px;width:-5px` printed `width:-5px`) (#42104). 24 real stylesheets - Print numbers from the f32's shortest digits, not its f64 expansion (#38565). 17 real stylesheets - Print negative numbers below 1 with a single minus sign (#38516) - Keep math functions over two different length units as written (#38489) - Fold rem() with a truncated quotient, so it keeps the sign of the dividend (#32286) - A one-argument `min()` / `max()` around a sum printed without any function (`width:100% - 20px`) (#46145) - `round()` breaks ties upward and handles a negative step as browsers do (#45249) **CSS modules** - A CSS module exports its `@keyframes` names (`styles.spin`). Hashing is unchanged (#46720). 145 real stylesheets - A name inside `:global(...)` prints plain but was exported as `name_<hash>`, a class no rule has (also #38573) (#45487). 90 real stylesheets - Keep container names global in CSS modules (#45247). 6 real stylesheets - Tag the keyframes name when an `animation` value falls back to raw tokens (#46034). 6 real stylesheets - In a plain stylesheet `composes: a from "./b.css"` was treated as an import: the file was inlined and an absolute path printed (#45961). 4 real stylesheets - Drop a rejected composes instead of failing the file at print time (#38538). 1 real stylesheets - Keep every rule's custom properties for the composes conflict check (#38523) - Keep composes working after a nested rule in the same block (#38520) - `IdentOrRef::hash` read 2 of 16 bytes, so in release builds every class of a module hashed alike (#30784) **Tokens, `url()`, diagnostics** - The block skip stack no longer allocates on every block exit (#39533) - Report the diagnostic's line without its line breaks in position.lineText (#38342). 192 real stylesheets - Compare `url()` values by their text (#46895). 26 real stylesheets - Values kept as tokens keep the blanks they were written with instead of having them re-invented at print time: `attr(x type(<number>))` printed `type(< number> )`, which browsers drop. Minified output still drops the blanks that cannot matter (#45577). 19 real stylesheets - The IE star hack (`*zoom:1`) failed the whole build. The declaration is skipped, as every browser does. It is the one syntax error that builds: tachyons, handsontable and jasmine ship it (#46751). 8 real stylesheets - The path of a copied asset spliced into `url()` is always quoted, so a file name with a space or a parenthesis cannot end the `url()` (#41557). 7 real stylesheets - Move the skip loop of parse_until_before out of the generic body (#39520) - Remove dead code from bun_css: the unused `impl Default for Property` (#45618) - Default the at-rule and qualified-rule parser traits, drop the url_to_css shim (#31999) **Speed** Four inputs that took quadratic or exponential time, or produced output out of proportion, are now linear, with no new limit and no extra pass (#31639, #42243, #38531, #43421). **Tests only** - `css-fuzz.test.ts` never ran in CI and timed out on debug builds; it is a deterministic table now (#38500) - `nth-anplusb-ident.test.ts` counts heaps in a child process (#46331) - `color.test.ts` checks the ansi-16 sweep with one `expect()` (#42824) - Not CSS, but it failed this PR's CI and fails on `main` too: `ws-proxy.test.ts` on Windows, "a message sent during a TLS renegotiation arrives after it". The helper server now renegotiates from the callback of the frame's write (#46170) **No pull request covered these** - `--v: 5.0.1` printed as `5.1`, `--w: 1.0.0` as `10`, `--x: 1+2` as `12`, `2n+1` as `2n1`: a number token that touches another keeps its source text. - `color-scheme: auto` printed as `normal`, and `color-scheme: dark light` as `light dark`. - A bare `:global` or `:local` in a CSS module sets the scope of the names after it, up to the end of the selector, as in css-loader. It was a build error. - The scrollbar states (`a:horizontal`, `::-webkit-scrollbar{&:horizontal{}}`) are valid in a plain compound, as in WebKit and Blink. - Without browser targets, `-webkit-box-align`, `-ms-flex-align` and the like were dropped next to their standard form (flatpickr, foundation-sites, materialize-css: 28 stylesheets), and `-webkit-fill-available` before `-moz-available` (beercss). - `(-o-min-device-pixel-ratio: 2/1)` printed as `2`. - `sin()`, `cos()` and `tan()` are exact at multiples of 90deg: `calc(100px * cos(90deg))` printed `-.00000437114px`. - A `display-p3` color at targets that have P3 but not the other `color()` spaces (Safari 10.1 to 14) got itself as a fallback. - `width: rem(-18px, 5px)` folds to a negative literal once `rem()` is right; it prints `0`. - Consecutive nested declarations rules, which arise when same-query `@media` blocks in one rule merge, are folded into one and count against the existing selector expansion limit as the `&{}` rules they replace did. #### Known and not fixed here - Four longhands are still folded into a `transition` the author did not write where the targets add a prefixed copy of a property (`mask-image` at Chrome 80): only the shorthand gives the copy its own times. - A later `background`, `box-shadow` or `border-*` declaration still loses to the `@supports` copy of an earlier `var()` value of the same property with a wide-gamut color. Same on `main`; #45560 is taken for the properties of the fallback handler and for unparsed values only. - A merged rule prints the unprefixed name in the passes of the rule it absorbed: `.d::-webkit-input-placeholder{}.d::-moz-placeholder{}` prints `::placeholder` for the second (#42106, not taken: it breaks the merge of prefix-equivalent rules). - The `border`, `outline` and `font` shorthands, `line-height`, `scroll-padding`, `background-size` and a few more still print a `calc()` that folds below zero as a negative literal. No real stylesheet has one. - `all` is not a property Bun knows, so `a{margin-top:1px;all:unset}` prints `all` first. Same on `main`. - A selector kept as raw tokens loses the blank before `*` when minified: `--sel: .dark *` prints `.dark*`. Same on `main`. - `width:calc(0% + 12px)` folds to `12px`, which lays out differently in a table cell. Same on `main`. - The `:dir()` / `:lang()` rules of a lowered logical property print before the nested rules of their rule, so a logical declaration written after a nested rule of equal specificity still loses to it. Same on `main`. - A fraction is still rounded to 6 significant digits (`12345.678` prints `12345.7`, `33.3333333333%` prints `33.3333%`). Keeping every digit would add up to 282 KB over the corpus. - `@keyframes "quoted"` is hashed in a module but not exported. - In a module, a keyframes name inside a `var()` fallback is not hashed: `animation: var(--enter, fade 1s)`. Same on `main`. - A `color-mix()` or relative color folded to a hex inside a gradient changes the interpolation from Oklab to sRGB (VS Code, clawhub). Printing `color(srgb ..)` instead needs a fallback for `mask-image`, `list-style-image` and custom properties at the default targets, which have none. - A comma list in `@container` still fails the build. - After a bare `:global`, names that are hashed at print time (`::view-transition-old(foo)`) are still hashed; inside `:global(...)` they are not. #### Pull requests in scope that do not land here - Duplicates of a pull request that lands here: #36961 (of #37979), #38478 (of #46662), #38480 (of #46733), #38482 (of #46715), #38491 (of #38500), #38508 (of #45201), #38513 (of #45362), #38573 (of #45487), #38640 (of #38500), #45525 (of #45247), #45589 (of #46756), #45970 (of #45971) - Already fixed on `main`: #38674 - Right, but for input nobody writes (0 of 10,116 stylesheets, and not new syntax): #31916, #38526, #38592, #39349, #41139, #41801, #42042, #42484, #43078, #45604, #45908, #46086, #46221, #46224, #46273, #46694, #46707, #46737, #46755, #46816, #46870, #46934 - Right or harmless, but not worth their code, their bigger output or the behavior they change: #30905, #31924, #33332, #34426, #36225, #38006, #38512, #38547, #38551, #38561, #38572, #38576, #38607, #38623, #38667, #38692, #42106, #42912, #42918, #43407, #43434, #44700, #45442, #45546, #45747, #45759, #45825, #45946, #46061, #46127, #46206 - Need a decision (see above), stay open: #40368, #40369, #44647, #44648, #45814 - Stays open: #45787 Out of scope: CSS linking and chunking in the bundler, the dev server, docs-only pull requests, and repository-wide sweeps that touch `src/css` in passing. Thanks to @mikamikasuki (#45247), @alii (#39520, #31999) and @oddharsh, whose measurements on #40369 and #41990 were used. Fixes #16727. Fixes #24770. Fixes #30772. Fixes #44704. Fixes #45089. Fixes #45237. Fixes #46872. ### How did you verify your code works? - The 35 CSS test files pass on a debug build, also with `FORCE_COLOR=1`. 600+ of the tests in this branch fail on a build of `main`. An earlier head of this branch passed all 179 CI jobs that ran. - The reproduction of each issue listed above was run on `main` (wrong) and on this branch (right). - Differential run over the 10,116 stylesheets, `main` against this branch, minified without targets and through `Bun.build` at the default targets: no stylesheet that builds on `main` fails; Chromium 156 compares the output with the input rule by rule (CSSOM, then computed values on a sized element in a standards-mode document, colors as painted pixels in sRGB and P3). The comparison has 195 control pairs that must read equal or different. For stylesheets that read differently on both builds, mostly because of lowering, the sets of differences were compared; the three new ones are two minifier fixtures that are wrong on both and one comparison artifact. - CSS modules are checked for consistency instead, since hashed names defeat the comparison: every reference points at a name the output defines, every export exists. - `Bun.color`: 59 inputs in 13 formats on both builds; every changed cell was read.
|
Understood. Two facts for a reopen:
|
Stacked on #46697. No user reported this.
Problem
backgroundorbackground-imagelist gets a vendor prefix on every layer of its fallback declaration.radial-gradient(circle at top left, red, blue)besideimage-set("a.png" 1x)gives-webkit-radial-gradient(circle at 0 0, red, #00f), -webkit-image-set(url("a.png") 1x).get_fallbacks_image(src/css/small_list.rs:87-150) ORs the prefixes that each layer needs, then prefixes every layer.Fix
prefix_helperdecides per layer. An unprefixed gradient takes the prefix only if it needs it.image-set()still takes-webkit-, never-moz-or-o-.VendorPrefix::from_name_stris removed.test/js/bun/css/css.test.ts(21 tests fail without the fix), other CSS suites. Self-reviewed: 36 concerns raised, 33 addressed (Notes).Background
at, no color hint. bun prints those parts in the standard grammar only.Image::get_prefixed: the union is in the list routine. Considered the same test forimage-set(): it loses-webkit-beside an author-webkit-gradient. lightningcss has the same union.Downsides
-webkit-linear-gradient(..), linear-gradient(..)gets no all--webkit-copy now, and Chrome 10 to 25 read only that copy..text: +256 bytes.flushwith no background: 20 instructions (was 18).get_fallbacksfor a list that needs no prefix: 165 (was 282).Notes
The base of this PR is the branch of #46697, because this PR changes five expectations that #46697 adds. The Rust applies to main as is. When #46697 merges, this branch moves onto main.
Repro (bun 1.4.3, 1.4.4, main 5c6d4e2, default browser targets):
CSS.supports('background-image', v)in headless Chrome 153, which still has the parser for the prefixed functions:The old copy is invalid when the gradient has
at <position>or a color hint. It is valid but drawn differently when a radial gradient has an explicit size:-webkit-radial-gradient(20px, red, blue)reads20pxas a position. #46697 corrects the direction of a linear copy. This change removes the copy in all of these cases.Reach. Of the default targets, Chrome and Edge 80 to 112 and Opera 67 to 98 need
-webkit-image-set(). #40369 raises the default targets to Chrome and Edge 111, so the case then remains for versions 111 and 112. With those targets the handler also drops an author-webkit-image-set()declaration that comes before the standard one and generates it again, so the copy reaches a correct hand-written fallback too. One row covers that. #40368 adds acssTargetoption, which makes the older targets reachable.The rule.
prefix_helperbuilds the declaration for one prefix and pushes it only when a layer changed. In that declaration:image-set()takes-webkit-in each-webkit-declaration, as before. It never takes-moz-or-o-.When no layer is unprefixed and a declaration was generated, the last one replaces the written list, as before.
linear-gradient(to right,red,blue), image-set("a.png" 1x)-webkit-linear-gradient()linear-gradient(to right, ...)radial-gradient(red,blue), image-set("a.png" 1x)-moz-image-set(...)and-o-image-set(...)-moz-radial-gradient(...), image-set(...)and-o-radial-gradient(...), image-set(...)-webkit-linear-gradient(left,red,blue), url(a.png)-moz-linear-gradient(left,red,blue), image-set("a.png" 1x)-webkit-linear-gradient(left, ...), -webkit-image-set(...), then-moz-linear-gradient(...), -moz-image-set(...), then the value-webkit-linear-gradient(left, ...), -webkit-image-set(...), then the value-webkit-linear-gradient(left,red,blue), -moz-linear-gradient(left,red,blue)-webkit-linear-gradient(left,red,blue), image-set("a.png" 1x)-webkit-linear-gradient(left, ...), -webkit-image-set(...), then the valuelinear-gradient(red,blue), image-set("a.png" 1x)-webkit-linear-gradient(...), -webkit-image-set(...), then the valueTests. Under
background: 26 rows, 4 tests that check each pair of 11 layers (no declaration twice, no-moz-image-set()or-o-image-set()), and 1bun buildtest for the repro. #46697 has rows that pin the copy this PR removes at the default targets. Three of them move to Chrome 25, where the gradient itself needs-webkit-, with the same expected output. Thelayersstrings of itsbun buildtest now expect the unprefixed gradient. Without the fix, 21 tests fail: 15 of the rows, the 4 pair tests and the 2bun buildtests.Measurements. Base is the branch of #46697 (6494118). Rows come from
cssInternals.prefixTest. The per-call numbers come from release builds of both trees (gdb onbun-profile, third call of a loop).image-set(): 0 of 54 rows differ.test/js/bun/css/filesdiffer underbun build. 0 of 591 rows differ for the 197.cssfiles of the checkout at 3 target sets.CSS.supports), 74 mixed-list rows at the default targets: the first declaration is accepted in 66 (was 56). Without the declarations that have an unprefixedimage-set(), which Chromium 80 to 112 cannot read, 66 rows have an accepted declaration (was 58). With no targets the two counts are 62 (was 60) and 30 (was 30). No row loses an accepted declaration.-moz-image-set()or-o-image-set()0 (was 114). Rows with the same declaration twice 10 (was 76). All 10 have a target that needs-webkit-gradient(). No row has more declarations than before. No row loses every-webkit-image-set()that it had.-webkit-linear-gradient(left,red,blue), linear-gradient(red,blue)at the default targets had an all--webkit-copy. Chrome 10 to 25 read that copy, and they are not targets.-webkit-gradient()(chrome 8, safari 6, chrome 8 + firefox 4 + opera 11.5): the-webkit-gradient()declarations differ in 0 of 372 rows.url(b.png), image-set("a.png" 1x)2>2>2>2 (was 2>3>4>5).radial-gradient(...), -webkit-image-set(...)1>1>1>1 (was 2>3>5>9).-webkit-linear-gradient(left,red,blue), url(a.png)1>1>1>1 (was 2>3>4>5). The repro 2>2>2>2 (was 2>2>2>2)..text: 82,259,017 bytes (was 82,258,761). The stripped binary keeps its size.get_fallbacks::<Background>2755 B (was 2610),BackgroundHandler::flush9520 B (was 9244), the twoprefix_helpercopies 926 B and 772 B (were 848 and 807). With the pass inline in the loop, an earlier revision grew.textby 3,840 bytes, soprefix_helperis#[inline(never)].get_fallbacksforbackground:url(a.png)165 (was 282), indirect jumps 0 (was 3).BackgroundHandler::flushforbackground-image:url(a.png)416 (was 527). Aflushwith no background property: 20 (was 18), because the compiler now stores two arguments before the early return.mi_mallocfamily calls per flush:background:-webkit-linear-gradient(left,red,blue)7 (was 8). The same asbackground-image3 (was 4).background:url(b.png),image-set("a.png" 1x)8 (was 8).background:url(a.png)1 (was 1).Targets::prefixescalls per flush: the repro 4 (was 2). A list with three prefixed declarations 8 (was 2). Aurl()list 0 (was 0).Self-review, not taken
image-set()rule limited to a list in which an unprefixed layer needs-webkit-. A--target=bunbuild of-webkit-linear-gradient(left,red,blue), image-set("a.png" 1x)then loses the-webkit-image-set()declaration that main prints.Related PRs
prefix_helperand also deletesVendorPrefix::from_name_str. It conflicts textually with this PR.at <position>as tokens. No row of this PR has such a value as input.css.test.tspasses on a tree that has both PRs (1650 tests).impl FallbackValues for SmallVec<[T; 1]>). A search found no upstream report of it, and this PR files none.Not changed here
-webkit-gradient()declaration, for targets such as Chrome 9 or Safari 6. No release build can select such targets. A list with no gradient to convert still gets a copy of itself there (Duplicate 'image-set' generated with certain browserslist targets parcel-bundler/lightningcss#436). The single-image path (Image::get_fallbacks, used byborder-image) has the same copy. That declaration still leaves out a-webkit-gradient()that the author wrote.BackgroundHandlerkeeps each declaration that has a prefixed layer in a stash and also flushes it (background.rs:816-824,:1139-1151). A list that keeps an author-prefixed layer beside a layer that needs a generated prefix still gains a declaration when a same-selector rule merges:-moz-linear-gradient(left,red,blue), image-set("a.png" 1x)gives 2 declarations, then 3 after the merge (was 3, then 5). lightningcss does the same.image-set()needs Firefox 88 against the default Firefox 78 (flush_helper!,background.rs:619-630). The later value then gets no generated fallback.background-image: image-set("a.png" 1x); background-image: image-set("b.png" 1x)gives-webkit-image-set()fora.pngonly. css: keep an earlier gap or flex-basis declaration when a target lacks the later one #46694, css: keep border-image andborderin source cascade order within a rule #42018 and css: keep an earlier transform or translate declaration when a target does not support the later one #45908 change the same policy in other handlers.image-set()withtype()or a gradient option still gets a-webkit-image-set()copy with those parts. A comment inImageSetOption::to_csssays that-webkit-image-set()does not support them.var()layer) and thelist-style-image,cursorandcontentproperties do not pass through this routine. They get no-webkit-image-set()copy.[human-review] gate passed · iteration 1 · 3 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 1
evidence per changed file