[robobun]

Problem

• When a rule sets all four logical border sides and the two block sides are equal, the border minifier folds the inline sides into a border-inline-* shorthand. It does that even when the targets do not support logical properties and every other logical border declaration in the sheet is being compiled to physical properties, so the target browsers drop the declaration and the inline sides end up with the block sides' border (released 1.4.0 and main, targets: { chrome: 60 }):

  .a{border-block-start:3px solid red;border-block-end:3px solid red;border-inline-start:1px solid red;border-inline-end:1px solid red}
    -> .a{border:3px solid red;border-inline-width:1px}
  .a{... same block sides ...;border-inline-start:1px solid red;border-inline-end:2px solid red}
    -> .a{border:3px solid red;border-inline-width:1px 2px}
  .a{... same block sides ...;border-inline-start:1px dotted blue;border-inline-end:1px dotted blue}
    -> .a{border:3px solid red;border-inline:1px dotted #00f}
  

  Targets that support the longhands but not the shorthands (chrome 69-86, safari 12.1-14.0, firefox 41-65) get the same output.
• Cause: flush_category! in src/css/properties/border.rs, the top_eq_bottom arm (lines 1068-1118 on main). It emits BorderInlineWidth / BorderInlineStyle / BorderInlineColor (one sub-property differs) or BorderInline (several differ, inline sides equal) guarded only by is_logical. Every other logical shorthand emitted by this macro is guarded by f.logical_supported / f.logical_shorthand_supported, and fc_prop! has no compiling arm for these four properties (it compiles the single-side longhands only), so they are pushed to the output as written.
• Upstream lightningcss 1.30.2 produces the same output for these inputs, so this is inherited rather than a porting error.

Fix

• The arm now emits through the same target-aware paths the rest of the flush uses for the same values:
  • shorthand supported: unchanged, border-inline-<sub>: start end via logical_shorthand!, or border-inline for whole sides;
  • one sub-property differs and both inline values are equal: the physical pair via inline_prop! (border-left-width: 1px; border-right-width: 1px), which is direction independent and what the handler already emits for border-inline-width: 1px on its own;
  • otherwise the arm reports itself unhandled and the existing side_diff! fallback compiles each inline side (logical longhands when the targets have them, :lang() ltr/rtl rules otherwise), which is what it already did for unequal whole sides.
• Whole equal sides now go through equal_inline_sides!, which is the block that the not-all-sides-set path below already used (border-inline, border-inline-start + border-inline-end, or border-left + border-right depending on the targets), moved into a macro so both sites share it. That path's output is unchanged.
• Why this is correct: inside this arm both inline sides are known to differ from the block sides (the arms above handle the cases where one of them does not), so when exactly one sub-property differs, it differs on both sides and side_diff! emits exactly that sub-property per side; when more than one differs and the sides are equal, every form equal_inline_sides! emits carries the whole side. Every declaration the arm now emits is one the handler already emits for the same targets elsewhere, and the :lang() rules are only produced when the two inline values actually differ. The physical instantiation of the macro (is_logical = false) never enters this code, and inline_prop! only gained a boolean result.
• Verified:
  • test/js/bun/css/css.test.ts, new border-inline shorthands for compiled targets block: safari 8 (nothing logical supported) for each of width / style / color with equal inline values, the whole-side case, and the unequal width and color cases that need :lang() rules; safari 13 (longhands only) for the equal width and the unequal width / style cases plus the whole-side case; safari 15 pins the unchanged shorthand output for all four shapes. 10 of the 14 cases fail on the unfixed build, all pass with the fix.
  • bun bd test test/js/bun/css/css.test.ts: 1156 pass; test/bundler/css/ and test/bundler/esbuild/css.test.ts: 225 pass.
  • cargo clippy -p bun_css and cargo fmt are clean.

Background

• The CSS minifier buffers all border declarations of a block in BorderHandler and writes them out in flush, as the shortest equivalent set it can find. flush_category! is the shared body for the physical sides (is_logical = false) and the logical sides (is_logical = true); the arm touched here handles "top and bottom equal, left and right differ", emitting border: for the block sides and then only the inline differences.
• Logical properties (border-inline-start, ...) pick a physical side based on the writing direction. For targets that predate them the handler compiles them away: a value that is the same on both inline sides becomes plain left/right properties, anything else becomes one extra rule per direction, selected with :lang() lists (PropertyHandlerContext::add_logical_rule). Two compat features drive this: LogicalBorders for the single-side longhands and LogicalBorderShorthand for the two-sided border-inline* / border-block* shorthands, exposed to the flush as logical_supported and logical_shorthand_supported.
• fc_prop! is the emission macro: for the single-side logical properties it has arms that compile to the physical property or the direction rules; for everything else it just pushes the declaration. Call sites that want a two-sided shorthand therefore have to check the flags themselves, which is what this arm was missing.

Output after the fix for the three inputs above

chrome 60 (nothing logical supported)
  .a { border: 3px solid red; border-left-width: 1px; border-right-width: 1px; }
  .a { border: 3px solid red; }
  .a:not(:-webkit-any(:lang(ae), ...)) { border-left-width: 1px; border-right-width: 2px; }
  .a:not(:is(:lang(ae), ...))          { border-left-width: 1px; border-right-width: 2px; }
  .a:-webkit-any(:lang(ae), ...)       { border-left-width: 2px; border-right-width: 1px; }
  .a:is(:lang(ae), ...)                { border-left-width: 2px; border-right-width: 1px; }
  .a { border: 3px solid red; border-left: 1px dotted #00f; border-right: 1px dotted #00f; }

chrome 80 (longhands supported, shorthands not)
  .a { border: 3px solid red; border-left-width: 1px; border-right-width: 1px; }
  .a { border: 3px solid red; border-inline-start-width: 1px; border-inline-end-width: 2px; }
  .a { border: 3px solid red; border-inline-start: 1px dotted #00f; border-inline-end: 1px dotted #00f; }

chrome 90 (everything supported, unchanged)
  .a { border: 3px solid red; border-inline-width: 1px; }
  .a { border: 3px solid red; border-inline-width: 1px 2px; }
  .a { border: 3px solid red; border-inline: 1px dotted #00f; }

Also checked: the !important variant, the same sides written as border-block + border-inline, style attributes (logical properties are never compiled there, output unchanged), and the left_eq_right arm next to this one, which does not emit logical shorthands and is unchanged. A lab() inline color after border: gets no rgb fallback on the new physical path; that matches what the physical handler already does for border: ...; border-right-color: lab(...) (the preceding border: value is the fallback), so nothing new there.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 29 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dfb27098-68e5-4665-84ae-e9ad935788af

📥 Commits

Reviewing files that changed from the base of the PR and between eabb96d and 75009e8.

📒 Files selected for processing (2)

• src/css/properties/border.rs
• test/js/bun/css/css.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status

• Reproduced on released 1.4.0 and on main (eabb96d) with cssInternals.prefixTest and targets: { chrome: 60 }: the four-logical-sides inputs in the description come out as border-inline-width / border-inline even though the targets get logical borders compiled away. Upstream lightningcss 1.30.2 gives the same output, so the bug is inherited.
• Fix is in src/css/properties/border.rs (flush_category!, the top_eq_bottom arm); tests are the new border-inline shorthands for compiled targets block in test/js/bun/css/css.test.ts: 10 of the 14 cases fail without the fix, all 14 pass with it, and the rest of test/js/bun/css plus the bundler CSS tests are unchanged.
• Waiting on CI.

[claude]

LGTM — targeted fix that gates the border-inline* shorthands this arm produces on logical_shorthand_supported, matching every other logical emission in flush_category!.

What was reviewed:

• Traced the diff == 1 restructure: inside this arm both inline sides differ from the block sides, so when only one sub-property differs it differs on inline_start too — the else (color) branch is reachable exactly when the old else if was.
• inline_prop! now returns a bool; its three original call sites drop it (bool is not #[must_use]), and it only .take()s when it emits, so the handled = false fallback still sees intact fields.
• equal_inline_sides! is a byte-for-byte extraction of the existing not-all-sides-set block; diff > 1 && left_eq_right is equivalent to the old three-field equality check via BorderShorthand::eql.
• 14 new prefix_test cases cover safari 8/13/15 × width/style/color/whole-side × equal/unequal, including the :lang() fallback path.

Extended reasoning...

Overview

Fixes the top_eq_bottom arm of flush_category! in src/css/properties/border.rs, which was emitting border-inline-width/-style/-color/border-inline shorthands unconditionally when minifying four logical border sides with equal block sides. Unlike every other logical shorthand in this macro, these were not gated on f.logical_shorthand_supported, and fc_prop! has no compiling arm for them, so old targets received declarations they drop. The fix adds two small local macros (inline_diff!, equal_inline_sides!) that route through the same target-aware paths already used elsewhere in the flush, and lets the existing side_diff! fallback handle the unequal-values case. inline_prop! gains a boolean result. The second callsite of equal_inline_sides! is a pure extraction of an existing block.

Security risks

None. This is CSS minifier output shaping — no untrusted-input parsing, allocation sizing, or FFI is touched. Worst case is incorrect CSS output, which the tests pin.

Level of scrutiny

Medium. The macro is dense, but the change is confined to one arm of one instantiation (is_logical = true; the physical instantiation never enters the modified block). I traced each new path against the invariants established by the earlier arms: since top_eq_left and top_eq_right are both false here, both inline sides differ from the block sides, so when diff == 1 the differing sub-property is guaranteed to differ on inline_start, making the new else branch equivalent to the old explicit color check. The diff > 1 && left_eq_right condition is equivalent to the old three-field equality check because left_eq_right is inline_start.eql(inline_end). inline_prop! only .take()s when it emits, so the unhandled fallback still sees intact values.

Other factors

Test coverage is thorough: 14 prefix_test cases across three target tiers (nothing supported / longhands only / everything supported), exercising each of width/style/color individually, the whole-side case, and the unequal-values :lang() fallback. The safari 15 cases pin the unchanged shorthand output. The PR description reports 10/14 fail on the unfixed build and the full css test suite plus bundler css tests pass. No CODEOWNERS cover src/css/. No prior human review comments to address.

[Jarred-Sumner]

Not taken in #47025 (7942008), which consolidated the open CSS parser pull requests. The fix is right, but none of the 10,116 real stylesheets it was checked against has this input, and it is not new syntax. Reopen it if the input shows up in real CSS.