[number field] Reject implausibly grouped input text - #5425
Conversation
Raw text entry (drop, IME composition, autofill) accepted strings like 1.2.3 that parseNumber collapses to 12.3, so the input displayed one number while the value and the hidden input submitted another until blur. Validate the separator structure on the change path: multi-dot strings are only accepted when they read as European-style grouping (1.234.567.89), keeping the mixed-locale normalization intact. Fixes mui#5424
Bundle size
PerformanceTotal duration: 1,249.16 ms -179.23 ms(-12.5%) | Renders: 78 (+0) | Paint: 1,962.24 ms -275.67 ms(-12.3%)
…and 1 more (+9 within noise) — details Metric alarms
Check out the code infra dashboard for more information about this PR. |
✅ Deploy Preview for base-ui ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f01e625077
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| locale?: Intl.LocalesArgument, | ||
| options?: Intl.NumberFormatOptions, | ||
| ) { | ||
| const { unformatted } = unformatNumber(text, locale, options); |
There was a problem hiding this comment.
Validate grouping before locale normalization
For locale="de-DE"—and when an omitted locale resolves to one using . for grouping—unformatNumber removes every dot before this function inspects the runs. Consequently, isPlausibleNumberInput('1.2.3', 'de-DE') sees 123 and returns true; NumberField explicitly permits repeated group symbols, so this text remains visible while the hidden input becomes 123, bypassing the guard intended to reject implausible grouping. Validate raw locale-group runs before stripping them.
Useful? React with 👍 / 👎.
| const { unformatted } = unformatNumber(text, locale, options); | ||
|
|
||
| // Only the mantissa can contain grouping; the exponent is parsed as-is. | ||
| const [mantissa] = unformatted.split(/e/i); |
There was a problem hiding this comment.
Validate separators after the exponent marker
With scientific notation, a raw change such as 1E2.3.4 passes because this split discards the exponent from validation. parseNumber then collapses the dots into 1e23.4, which parseFloat reads as 1e23, while the input continues displaying 1E2.3.4; autofill, drop, and IME paths can therefore retain the same visible/hidden-value divergence this guard targets. Validate the exponent structure as well, or reject separators within it.
Useful? React with 👍 / 👎.
| if (!isPlausibleNumberInput(targetValue, locale, formatOptionsRef.current)) { | ||
| return; |
There was a problem hiding this comment.
Reset dirty state when rejecting the change
When this new guard rejects otherwise character-valid input on an untouched field, allowInputSyncRef has already been set to false and is not restored. Blurring then treats the unchanged field as manually edited and calls onValueCommitted with its old value; a controlled value update before blur is also skipped by the root's input-sync effect, leaving stale displayed text. Restore the sync flag before returning when no input text was accepted.
Useful? React with 👍 / 👎.
Raw text entry (drop, IME composition, autofill) accepted strings like
1.2.3, whichparseNumbercollapses to12.3. The input kept showing1.2.3whilevalueand the hidden input held12.3until blur, so a form submitted in between sent a number the user never saw.NumberField.Inputnow rejects change-event text whose separator structure is not plausible for the locale, next to the existing character check. Multi-dot strings are only accepted when they read as European-style grouping.1.234.567.89and mixed-locale paste normalization behave as before.parseNumberitself is unchanged.1..5was previously accepted from raw text entry and parsed as1.5; it is now rejected there. On blur and paste it still normalizes to1.5.Fixes #5424