Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions packages/react/src/number-field/input/NumberFieldInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { useLabelableContext } from '../../internals/labelable-provider/Labelabl
import {
getNumberLocaleDetails,
isNumeralChar,
isPlausibleNumberInput,
parseNumber,
ANY_MINUS_RE,
ANY_PLUS_RE,
Expand Down Expand Up @@ -271,6 +272,16 @@ export const NumberFieldInput = React.forwardRef(function NumberFieldInput(
return;
}

// Reject text whose separator structure `parseNumber` would silently reinterpret
// (e.g. `1.2.3` -> 12.3). Accepting it would keep the raw text visible while the
// numeric value and the hidden input diverge from it until blur, so a form could
// submit a number the user never saw. Grouping-like strings (`1.234.567.89`)
// remain accepted. Typing can't produce these, since `onKeyDown` blocks a
// second decimal separator, but drop, IME composition, and autofill land here.
if (!isPlausibleNumberInput(targetValue, locale, formatOptionsRef.current)) {
return;
Comment on lines +281 to +282

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

}

const parsedValue = parseNumber(targetValue, locale, formatOptionsRef.current);

setInputValue(targetValue);
Expand Down
27 changes: 27 additions & 0 deletions packages/react/src/number-field/root/NumberFieldRoot.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2627,6 +2627,33 @@ describe('<NumberField />', () => {
expect(onValueChange.mock.calls[0][0]).toBe(1234567.89);
});

it('rejects implausibly grouped text instead of diverging from the visible value', async () => {
// `1.2.3` is not a plausible number in any locale, but `parseNumber` collapses it
// to 12.3. Accepting it would display `1.2.3` while the value and the hidden
// input submit 12.3 until blur.
const onValueChange = vi.fn();

await render(<NumberField name="n" defaultValue={5} onValueChange={onValueChange} />);

const input = screen.getByRole('textbox');
const hiddenInput = document.querySelector<HTMLInputElement>(
'input[type="number"][name="n"]',
)!;

fireEvent.change(input, { target: { value: '1.2.3' } });

expect(onValueChange.mock.calls.length).toBe(0);
expect(input).toHaveValue('5');
expect(hiddenInput).toHaveValue(5);

// Plausible grouping is still accepted through the same path.
fireEvent.change(input, { target: { value: '1.234.567.89' } });

expect(onValueChange.mock.calls.length).toBe(1);
expect(onValueChange.mock.calls[0][0]).toBe(1234567.89);
expect(input).toHaveValue('1.234.567.89');
});

it('allows composition key events (IME) without preventing default', async () => {
await render(<NumberField />);

Expand Down
44 changes: 43 additions & 1 deletion packages/react/src/number-field/utils/parse.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { expect } from 'vitest';
import { isJSDOM } from '#test-utils';
import { getNumberLocaleDetails, isNumeralChar, parseNumber } from './parse';
import {
getNumberLocaleDetails,
isNumeralChar,
isPlausibleNumberInput,
parseNumber,
} from './parse';

describe('NumberField parse', () => {
describe('getNumberLocaleDetails', () => {
Expand Down Expand Up @@ -323,6 +328,43 @@ describe('NumberField parse', () => {
});
});

describe('isPlausibleNumberInput', () => {
it('accepts plain and single-separator input', () => {
expect(isPlausibleNumberInput('1234')).toBe(true);
expect(isPlausibleNumberInput('1.5')).toBe(true);
expect(isPlausibleNumberInput('1,234.56')).toBe(true);
expect(isPlausibleNumberInput('12.')).toBe(true);
expect(isPlausibleNumberInput('.5')).toBe(true);
expect(isPlausibleNumberInput('12%')).toBe(true);
});

it('accepts partial input that does not parse yet', () => {
expect(isPlausibleNumberInput('')).toBe(true);
expect(isPlausibleNumberInput('-')).toBe(true);
});

it('accepts European-style dot grouping', () => {
expect(isPlausibleNumberInput('1.234.567.89')).toBe(true);
expect(isPlausibleNumberInput('1.234.567')).toBe(true);
expect(isPlausibleNumberInput('1.234.')).toBe(true);
expect(isPlausibleNumberInput('1.234.567,89', 'fr-FR')).toBe(true);
});

it('rejects multi-dot input that is not grouping-like', () => {
expect(isPlausibleNumberInput('1.2.3')).toBe(false);
expect(isPlausibleNumberInput('1..5')).toBe(false);
expect(isPlausibleNumberInput('....5')).toBe(false);
expect(isPlausibleNumberInput('1234.567.89')).toBe(false);
});

it('applies the locale decimal separator when checking', () => {
// de-DE: `.` is grouping (stripped) and `,` is the decimal, so repeated
// commas hit the same keep-last-dot collapse.
expect(isPlausibleNumberInput('1.234,56', 'de-DE')).toBe(true);
expect(isPlausibleNumberInput('1,2,3', 'de-DE')).toBe(false);
});
});

describe('isNumeralChar', () => {
it('accepts a digit from every supported numeral system', () => {
// ASCII, Arabic-Indic, Persian, fullwidth, and Han (including both zero forms).
Expand Down
55 changes: 53 additions & 2 deletions packages/react/src/number-field/utils/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,12 @@ export function getNumberLocaleDetails(
return { ...result, decimal };
}

export function parseNumber(
/**
* Runs the locale-aware normalization shared by `parseNumber` and
* `isPlausibleNumberInput`: separators, symbols, and numerals are reduced to an
* ASCII string where `.` is the only remaining separator candidate.
*/
function unformatNumber(
formattedNumber: string,
locale?: Intl.LocalesArgument,
options?: Intl.NumberFormatOptions,
Expand Down Expand Up @@ -183,10 +188,56 @@ export function parseNumber(
[HAN_RE, (ch: string) => String(Math.max(HAN_NUMERALS.indexOf(ch) - 1, 0))],
];

let unformatted = replacements.reduce((acc, [regex, replacement]) => {
const unformatted = replacements.reduce((acc, [regex, replacement]) => {
return regex ? acc.replace(regex, replacement as any) : acc;
}, input);

return { input, unformatted, isNegative };
}

/**
* Whether raw input text keeps its meaning through `parseNumber`'s
* keep-last-dot collapse. Multiple surviving `.` separators are plausible only
* when they read as European-style grouping (`1.234.567.89`); anything else
* (`1.2.3`) would parse to a number whose decimal placement differs from the
* visible text, so live text entry should reject it rather than let the
* display and the parsed value diverge.
*/
export function isPlausibleNumberInput(
text: string,
locale?: Intl.LocalesArgument,
options?: Intl.NumberFormatOptions,
) {
const { unformatted } = unformatNumber(text, locale, options);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.


// Only the mantissa can contain grouping; the exponent is parsed as-is.
const [mantissa] = unformatted.split(/e/i);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

const runs = mantissa.split('.');

if (runs.length <= 2) {
return true;
}

const first = runs[0];
const fraction = runs[runs.length - 1];
const interior = runs.slice(1, -1);

return (
/^\d{1,3}$/.test(first) &&
interior.every((run) => /^\d{3}$/.test(run)) &&
(fraction === '' || /^\d+$/.test(fraction))
);
}

export function parseNumber(
formattedNumber: string,
locale?: Intl.LocalesArgument,
options?: Intl.NumberFormatOptions,
) {
const result = unformatNumber(formattedNumber, locale, options);
const { input, isNegative } = result;
let { unformatted } = result;

// Mixed-locale safety: keep only the last '.' as decimal
const lastDot = unformatted.lastIndexOf('.');
if (lastDot !== -1) {
Expand Down
Loading