Skip to content

Fix radial-gradient position being dropped after an explicit size - #57873

Closed
Titozzz wants to merge 1 commit into
mainfrom
titozzz/fix-radial-gradient-position-after-explicit-size
Closed

Fix radial-gradient position being dropped after an explicit size#57873
Titozzz wants to merge 1 commit into
mainfrom
titozzz/fix-radial-gradient-position-after-explicit-size

Conversation

@Titozzz

@Titozzz Titozzz commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Summary:

processBackgroundImage silently drops the at <position> clause of a radial gradient whenever it follows an explicit size, and then re-parses the position values as a new size that overrides the declared one.

In the explicit-size branch of parseRadialGradientCSSString, the parser shifts the next token to look for a second size value. When that token is not a length/percentage it is discarded instead of being put back — so for radial-gradient(circle 100px at 25% 75%, red, blue) the at token is swallowed, the loop then treats 25% and 75% as a new <size>, and the gradient parses as:

// before
{shape: 'circle', size: {x: '25%', y: '75%'}, position: {top: '50%', left: '50%'}}
// after (matches web)
{shape: 'circle', size: {x: 100, y: 100}, position: {left: '25%', top: '75%'}}

The bug was invisible in tests because the only existing test for this syntax uses at center, which is indistinguishable from the default position.

The fix is to unshift the peeked token back so the main loop processes it (this also fixes <size> <shape> orderings like 100px ellipse, where the shape keyword was previously swallowed too). The structured C++ parser in react/renderer/css/CSSBackgroundImage.h is not affected — this is specific to the JS tokenizer.

Changelog:

[GENERAL] [FIXED] - Fix radial-gradient at <position> being ignored (and corrupting the size) when it follows an explicit size

Test Plan:

Added two Fantom tests in processBackgroundImage-itest.js covering circle 100px at 25% 75% (single explicit size + position) and 50px 100px at left bottom (two sizes + keyword position).

Verified the parse output for a matrix of radial gradient strings against Chrome's accepted/computed values (all match after the fix, including the untouched circle 100px at center case covered by the existing test):

input before after
circle 100px at 25% 75% size {25%, 75%}, position center size {100, 100}, position {left 25%, top 75%}
50px 100px at left bottom unaffected unaffected
circle 100px at center ✓ (masked the bug)

🤖 Generated with Claude Code

In processBackgroundImage, the explicit-size branch shifts the next token
to look for a second size value and discards it when it is not a length or
percentage. When that token is 'at', the whole position clause is lost:
the position defaults back to center and the position values are then
re-parsed as a new size, silently overriding the declared one.

radial-gradient(circle 100px at 25% 75%, red, blue) previously parsed as
size {x: '25%', y: '75%'} with position {top: '50%', left: '50%'};
it now parses as size {x: 100, y: 100} with position {left: '25%', top: '75%'}.

The existing test for this syntax only used 'at center', which is
indistinguishable from the default position, so the bug was invisible.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 10, 2026
@Titozzz
Titozzz marked this pull request as ready for review August 10, 2026 12:29
Titozzz pushed a commit to Titozzz/react-native-reanimated that referenced this pull request Aug 10, 2026
Ports the two upstream react-native processBackgroundImage changes
(react/react-native#57873 and react/react-native#57874) into the
backgroundImage processor to keep both parsers in sync:

- #57873 (position dropped after an explicit size) was already fixed
  here; align the code comment with upstream and use a px-sized circle
  in the regression test
- #57874: reject a percentage radius for circle radial gradients
  (explicit 'circle 50%' and the inferred circle from a single '50%'
  size). Per the CSS spec a circle radius must be a <length>;
  percentages remain valid for ellipse sizes

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011p2PjxNunbHriH5853qssK
@meta-codesync

meta-codesync Bot commented Aug 10, 2026

Copy link
Copy Markdown

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this in D115426012.

@meta-codesync meta-codesync Bot closed this in 66f27eb Aug 10, 2026
meta-codesync Bot pushed a commit that referenced this pull request Aug 10, 2026
Summary:
> [!NOTE]
> Stacked on #57873 (its commit is included here). Without that fix, `circle <length> at <position>` strings would mis-parse into percentage sizes and be wrongly rejected by this validation.

Per [css-images-3 `<radial-size>`](https://www.w3.org/TR/css-images-3/#valdef-radial-size-length-0), a circle's explicit radius must be a `<length>` — percentages are only valid for ellipses. Browsers reject the whole declaration for values like `radial-gradient(circle 50%, red, blue)` (computed `background-image: none`), while React Native accepted them and rendered an arbitrary interpretation (`max` of the value resolved against width and against height). The same style string therefore silently diverged between native and web, contradicting the "Same as web" validation policy this parser already follows for other invalid values.

`processBackgroundImage` now returns no gradient when a circle — explicit (`circle 50%`) or inferred from a single size (`radial-gradient(50%, ...)`, which browsers also reject) — has a percentage size. Ellipses with percentage sizes (`50% 20%`) are unaffected.

Note: the structured C++ parser in `react/renderer/css/CSSBackgroundImage.h` has the same leniency (`CSSRadialGradientExplicitSize` accepts `<length-percentage>` for both axes regardless of shape) and could get the same validation as a follow-up.

## Changelog:

[GENERAL] [FIXED] - Reject percentage radii for circle radial gradients, matching web behavior

Pull Request resolved: #57874

Test Plan:
Added three Fantom tests in `processBackgroundImage-itest.js`: `circle 50%` rejected, inferred-circle `50%` rejected, ellipse `50% 20%` still accepted.

Verified the accept/reject matrix against Chrome (`getComputedStyle(...).backgroundImage === 'none'` for rejected values):

| input | Chrome | RN before | RN after |
| --- | --- | --- | --- |
| `radial-gradient(circle 50%, red, blue)` | rejected | accepted | rejected |
| `radial-gradient(50%, red, blue)` | rejected | accepted | rejected |
| `radial-gradient(50% 20%, red, blue)` | accepted | accepted | accepted |
| `radial-gradient(circle 100px, red, blue)` | accepted | accepted | accepted |

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Reviewed By: fabriziocucci

Differential Revision: D115426055

Pulled By: cipolleschi

fbshipit-source-id: 5ba6e2f1d6e36bd5e64d7ef4f917918514f87d81
@meta-codesync meta-codesync Bot added the Merged This PR has been merged. label Aug 10, 2026
@meta-codesync

meta-codesync Bot commented Aug 10, 2026

Copy link
Copy Markdown

@cipolleschi merged this pull request in 66f27eb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants