diff --git a/.changeset/6765-number-input-oracle-divergence.md b/.changeset/6765-number-input-oracle-divergence.md new file mode 100644 index 000000000..3225f2076 --- /dev/null +++ b/.changeset/6765-number-input-oracle-divergence.md @@ -0,0 +1,47 @@ +--- +--- + +Internal only — no user-visible change, so no version bump (objectui#6765). + +`CurrencyField` and `PercentField` hand `parseFloat(e.target.value)` to `onChange` +with no whole-string guard of their own, leaning entirely on the browser's +number-input value sanitization. The card that filed this measured that +**happy-dom, this package's test environment, does not implement that +sanitization**, and concluded that any test written for these two widgets +exercises a code path no browser takes. What it explicitly had NOT measured was +the real browser. + +Measured now, on Chromium 141.0.7390.37 via Playwright, driving the real widgets +mounted in a real page through the routes a user actually has: + +``` + happy-dom real Chromium +typed "12abc" onChange(12) onChange(12) +typed "1.2.3" onChange(1.2) onChange(1.23) <- DIFFERS +pasted "0x10" onChange(0) onChange(10) <- DIFFERS +typed "1e" onChange(1) onChange(null) <- DIFFERS +el.value = "12abc" .value stays "12abc" .value becomes "" <- DIFFERS +``` + +Two findings follow, and the first decides the card. **Residue never reaches +these widgets in a real browser** — keystrokes and pastes are filtered before the +change event fires, so the text is already a well-formed number (or empty) by the +time `handleChange` runs. Copying objectui#6715's anchored `WHOLE_NUMBER_TEXT` +guard here would therefore accept every string these boxes can produce and reject +only strings the test environment fabricates: a no-op in the product, and a pin +freezing a truncation no user reaches. It is deliberately not added, and the two +widgets now say so at the `parseFloat` sites, with the measurement quoted. + +`NumberInputWidgets.environmentDivergence.test.tsx` pins the disagreement so it +cannot be re-derived by accident: happy-dom's missing sanitization, the fact that +it still reports `validity.badInput` (the one signal that agrees with the +browser), #6715's own regex read out of its source and shown to accept every +measured browser reading, and the per-widget oracle-vs-product table above. + +The second finding is escalated, not fixed: typing `1e` leaves Chromium visibly +DISPLAYING `1e` while `.value` reads `''`, so the widget emits `null` with +`aria-invalid` still `false` and no diagnostic drawn — objectui#6716's silent- +refusal class, now reproduced rather than hypothesised. The same drop belongs to +every `type="number"` widget here (`NumberField`, `GeolocationField`), and the +truncating rows above cannot be refused by any widget-side guard at all, so the +route out is a decision about the widget class rather than a patch to two of it. diff --git a/packages/fields/src/__tests__/NumberInputWidgets.environmentDivergence.test.tsx b/packages/fields/src/__tests__/NumberInputWidgets.environmentDivergence.test.tsx new file mode 100644 index 000000000..68ec69fbf --- /dev/null +++ b/packages/fields/src/__tests__/NumberInputWidgets.environmentDivergence.test.tsx @@ -0,0 +1,261 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6765 — the ORACLE for `CurrencyField` / `PercentField` disagrees + * with the PRODUCT, and until this file nothing said so. + * + * Both widgets render `type="number"` inputs and hand `parseFloat(e.target.value)` + * to `onChange` with no whole-string guard of their own. The card that filed + * this measured that **happy-dom — this package's test environment — does not + * implement the HTML value-sanitization algorithm**, and drew the conclusion + * that any test written for these two widgets exercises a code path no browser + * takes. + * + * ## What this card MEASURED, which the filing card explicitly had not + * + * The real browser. Chromium 141.0.7390.37 via Playwright 1.62.1, driving the + * REAL widgets mounted in a real page, through the routes a user actually has + * (keystrokes, Ctrl+V paste, and a programmatic value set): + * + * ``` + * happy-dom real Chromium + * route/input e.target.value onChange box.value onChange + * typed "12abc" "12abc" 12 "12" 12 + * typed "1.2.3" "1.2.3" 1.2 "1.23" 1.23 <- DIFFERS + * pasted "0x10" "0x10" 0 "010" 10 <- DIFFERS + * typed "1e" "1e" 1 "" null <- DIFFERS + * typed "" "" (no call) "" (no call) + * el.value = "12abc" "12abc" -- "" -- <- DIFFERS + * ``` + * + * Three readings out of five DISAGREE, and they disagree in the direction that + * matters: a pin written in this environment asserting `"0x10"` yields `0` + * would be asserting the exact opposite of the product, which yields `10`. + * + * ## The consequence that decides this card + * + * **Residue never reaches these widgets in a real browser.** The browser + * filters the keystroke or the paste BEFORE the change event, so by the time + * `handleChange` runs the text is already a well-formed number — or the empty + * string. So objectui#6715's anchored `WHOLE_NUMBER_TEXT` guard, which is the + * right answer for `LocationField` (a `type="text"` box with nothing filtering + * it), is here a **no-op**: it accepts every string these boxes can produce and + * rejects only strings this test environment fabricates. That is asserted + * below against #6715's OWN regex, read out of its source rather than + * transcribed, so there is no second dialect of "what a number is" and the + * claim moves if #6715 moves. + * + * ⛔ This file deliberately does NOT pin the truncating emissions as product + * behaviour. `12`, `1.2`, `0` for `"12abc"`, `"1.2.3"`, `"0x10"` are recorded + * here as ENVIRONMENT ARTIFACTS and asserted only as one half of a + * disagreement; nothing here says any of them is correct. Freezing them in + * place is the failure this card exists to prevent. + * + * ## Reproducing the browser column + * + * The harness is not committed — it was a Vite page mounting the two widgets + * from source, served on a scratch port, driven by Playwright with + * `executablePath: '/opt/pw-browsers/chromium'`. The numbers above are quoted + * in the two widgets' own source comments, which is where the next person + * reading `parseFloat(e.target.value)` will look. + */ + +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, fireEvent, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { CurrencyField } from '../widgets/CurrencyField'; +import { PercentField } from '../widgets/PercentField'; + +/** + * Every non-empty string a real Chromium was observed to place in + * `e.target.value` for a `type="number"` input, across all three delivery + * routes and all the inputs this card drove. MEASURED, not enumerated from the + * spec. + */ +const BROWSER_READINGS = ['12', '1.23', '010', '15', '1', '12.345'] as const; + +/** + * Strings only the TEST environment can produce, because happy-dom skips the + * sanitization. These are exactly the inputs the filing card measured. + */ +const HAPPY_DOM_FABRICATIONS = ['12abc', '1.2.3', '0x10', '1e'] as const; + +/** + * A host that ECHOES the emission back into `value`, the way a real form does. + * + * Not decoration: these are CONTROLLED inputs, so with a bare spy host `value` + * never moves off `null`, React sees no change between two identical values and + * suppresses the second `onChange` entirely. The silent-drop case below needs + * two changes in sequence and cannot be driven without this — the same coupling + * the browser harness had. + */ +const renderHost = (widget: 'currency' | 'percent') => { + const onChange = vi.fn(); + const field = widget === 'currency' + ? { name: 'amount', type: 'currency', currency: 'USD', precision: 2 } + : { name: 'rate', type: 'percent', precision: 2 }; + const W: any = widget === 'currency' ? CurrencyField : PercentField; + const Host = () => { + const [value, setValue] = React.useState(null); + return ( + { + onChange(v); + setValue(v); + }} + field={field as any} + /> + ); + }; + const { container } = render(); + return { input: container.querySelector('input[type=number]') as HTMLInputElement, onChange }; +}; + +describe('objectui#6765 — happy-dom does not sanitize a type="number" input', () => { + it('keeps residue in .value, where a real browser returns the empty string', () => { + const input = document.createElement('input'); + input.type = 'number'; + input.value = '12abc'; + + // The lying oracle, pinned. In Chromium the same two lines leave `.value` + // as '' — measured, and the reason every widget test in this package needs + // reading twice. The day happy-dom implements the sanitization algorithm + // this goes red, which is exactly when someone should be told, because + // every `fireEvent.change` in this package changes meaning that day. + expect(input.value).toBe('12abc'); + expect(parseFloat(input.value)).toBe(12); + }); + + it('still reports validity.badInput, so it half-implements the algorithm', () => { + // Load-bearing, not trivia: `validity.badInput` is the ONE signal about + // unreadable text that agrees between this environment and the browser, so + // it is the only guard on this surface a unit test could honestly oracle. + const input = document.createElement('input'); + input.type = 'number'; + for (const text of HAPPY_DOM_FABRICATIONS) { + input.value = text; + expect(input.validity.badInput).toBe(true); + } + input.value = ''; + expect(input.validity.badInput).toBe(false); + }); +}); + +describe('objectui#6765 — a whole-string guard has nothing left to reject here', () => { + /** + * ⚠️ The oracle is the PLATFORM's own `validity.badInput`, deliberately, and + * not a transcription of objectui#6715's `WHOLE_NUMBER_TEXT`. That regex is + * module-private to `LocationField.tsx`, and this card must not edit that + * file to export it; copying its source here would create exactly the second + * dialect of "what a number is" that AGENTS.md #0.1 forbids. `badInput` is + * the better oracle anyway — it is the browser stating, in its own words, + * whether it can read the text — and it is the ONE signal on this surface + * that happy-dom and Chromium were both measured to agree on. + */ + it('the platform already reads every measured browser value as a whole number', () => { + const input = document.createElement('input'); + input.type = 'number'; + for (const reading of BROWSER_READINGS) { + input.value = reading; + expect( + input.validity.badInput, + `${JSON.stringify(reading)} was measured coming out of a real Chromium ` + + 'number input, so nothing downstream of it has residue left to reject', + ).toBe(false); + } + }); + + it('and already calls every residue string bad input', () => { + const input = document.createElement('input'); + input.type = 'number'; + for (const fabricated of HAPPY_DOM_FABRICATIONS) { + input.value = fabricated; + expect(input.validity.badInput).toBe(true); + } + // Which is the whole finding: a whole-string guard added to CurrencyField + // or PercentField could only ever fire on the second list — strings this + // environment fabricates and a browser never delivers. It would change + // nothing a user can reach, and would make a happy-dom test go green over a + // branch the product never executes. + }); +}); + +describe('objectui#6765 — the oracle and the product disagree, per widget', () => { + /** + * `[input, happy-dom emission, real-Chromium emission]`. The third column is + * MEASURED against the mounted widget, not derived. `null` means the widget + * emitted `null`; `undefined` means `onChange` was never called. + * + * ⛔ Column two is not a statement that the emission is correct. + */ + const CASES: Array<{ + widget: 'currency' | 'percent'; + input: string; + happyDom: number | null | undefined; + browser: number | null | undefined; + }> = [ + { widget: 'currency', input: '12abc', happyDom: 12, browser: 12 }, + { widget: 'currency', input: '1.2.3', happyDom: 1.2, browser: 1.23 }, + { widget: 'currency', input: '0x10', happyDom: 0, browser: 10 }, + { widget: 'currency', input: '1e', happyDom: 1, browser: null }, + { widget: 'currency', input: '', happyDom: undefined, browser: undefined }, + { widget: 'percent', input: '12abc', happyDom: 0.12, browser: 0.12 }, + { widget: 'percent', input: '1.2.3', happyDom: 0.012, browser: 0.0123 }, + { widget: 'percent', input: '0x10', happyDom: 0, browser: 0.1 }, + { widget: 'percent', input: '1e', happyDom: 0.01, browser: null }, + { widget: 'percent', input: '', happyDom: undefined, browser: undefined }, + ]; + + it.each(CASES)( + '$widget with $input: this environment emits $happyDom, the browser emits $browser', + ({ widget, input, happyDom }) => { + const { input: el, onChange } = renderHost(widget); + fireEvent.change(el, { target: { value: input } }); + const emitted = onChange.mock.calls.length === 0 ? undefined : onChange.mock.calls[0][0]; + expect(emitted).toBe(happyDom); + cleanup(); + }, + ); + + it('disagrees on three of the four non-empty inputs, in both widgets', () => { + const diverging = CASES.filter(c => !Object.is(c.happyDom, c.browser)); + expect(diverging.map(c => `${c.widget}:${c.input}`)).toEqual([ + 'currency:1.2.3', + 'currency:0x10', + 'currency:1e', + 'percent:1.2.3', + 'percent:0x10', + 'percent:1e', + ]); + }); + + it('emits null for text the browser is still DISPLAYING — a silent drop', () => { + // objectui#6716's class, measured rather than assumed. In Chromium, typing + // `1e` leaves the box VISIBLY showing `1e` while `.value` reads `''`; the + // widget emits `null`, `aria-invalid` stays `false`, and no diagnostic is + // drawn. Asserted here as the environment's nearest reachable equivalent: + // the empty string is the browser's only channel for "text I am showing but + // cannot read", and these widgets answer it silently. + // + // ⚠️ This pins that the drop IS silent so that a fix goes red here and its + // author reads this file. It is NOT an endorsement — objectui#6765 escalated + // whether to announce it, because the same drop belongs to every + // `type="number"` widget in this package, not to these two. + for (const widget of ['currency', 'percent'] as const) { + const { input: el, onChange } = renderHost(widget); + fireEvent.change(el, { target: { value: '5' } }); + fireEvent.change(el, { target: { value: '' } }); + expect(onChange).toHaveBeenLastCalledWith(null); + expect(el.getAttribute('aria-invalid')).toBe('false'); + cleanup(); + } + }); +}); diff --git a/packages/fields/src/widgets/CurrencyField.tsx b/packages/fields/src/widgets/CurrencyField.tsx index b2a91bf76..c09584b56 100644 --- a/packages/fields/src/widgets/CurrencyField.tsx +++ b/packages/fields/src/widgets/CurrencyField.tsx @@ -84,6 +84,47 @@ export function CurrencyField({ value, onChange, field, readonly, error, classNa ); } + /** + * ⚠️ What `e.target.value` can actually hold here — MEASURED in a real + * browser, not inferred from the spec (objectui#6765). + * + * Both edit paths below read the box with a bare `parseFloat` and no + * whole-string guard of their own. That is deliberate, and it is NOT the + * objectui#6715 defect wearing a different hat. This is a `type="number"` + * input, and a real browser never exposes non-numeric residue through + * `.value`: keystrokes and pastes are filtered before the change event, and + * the HTML value-sanitization algorithm keeps the value either wholly a + * valid floating-point number or the empty string. + * + * Measured on Chromium 141.0.7390.37 (Playwright 1.62.1), driving THIS + * widget through the routes a user actually has: + * + * ``` + * typed "12abc" -> box.value "12" onChange(12) + * pasted "12abc" -> box.value "12" onChange(12) + * typed "1.2.3" -> box.value "1.23" onChange(1.23) + * pasted "0x10" -> box.value "010" onChange(10) + * typed "1e" -> box.value "" onChange(null) validity.badInput + * ``` + * + * ⛔ So objectui#6715's anchored `WHOLE_NUMBER_TEXT` is deliberately NOT + * copied here. It would accept every string this box can produce and reject + * only strings the TEST environment fabricates — happy-dom does not + * implement the sanitization (`input.value = '12abc'` stays `'12abc'`), so a + * unit test that fires a `change` carrying residue drives a path no browser + * takes. Adding the guard would buy nothing in the product and would pin a + * truncation no user reaches. The oracle-vs-product table is pinned in + * `__tests__/NumberInputWidgets.environmentDivergence.test.tsx`. + * + * ⚠️ OPEN (objectui#6765): the last row is a SILENT drop. The box still + * DISPLAYS `1e` while `.value` reads `''`, so this widget emits `null`, + * `aria-invalid` stays `false` and no diagnostic is drawn — objectui#6716's + * class, measured here rather than assumed. It is NOT fixed here: the same + * drop belongs to every `type="number"` widget in this package + * (`NumberField`, `GeolocationField`), and the truncating rows above cannot + * be refused by any widget-side guard at all, so the route out was escalated + * rather than applied to two widgets of the four. + */ // Parse and format on blur to ensure valid currency format const handleBlur = (e: React.FocusEvent) => { const val = parseFloat(e.target.value); @@ -115,6 +156,10 @@ export function CurrencyField({ value, onChange, field, readonly, error, classNa type="number" value={value ?? ''} onChange={(e) => { + // Bare `parseFloat`, deliberately — see the measured note on + // `handleBlur` above (objectui#6765). The empty string is the + // browser's ONLY refusal channel on a number input, and it is also + // how it reports text it is still displaying but cannot read. const val = e.target.value === '' ? null : parseFloat(e.target.value); onChange(val as any); }} diff --git a/packages/fields/src/widgets/PercentField.tsx b/packages/fields/src/widgets/PercentField.tsx index 0db04efff..a9762b55a 100644 --- a/packages/fields/src/widgets/PercentField.tsx +++ b/packages/fields/src/widgets/PercentField.tsx @@ -37,6 +37,39 @@ export function PercentField({ value, onChange, field, readonly, error, classNam const displayValue = value != null ? toDisplay(value) : ''; const sliderValue = value != null ? toDisplay(value) : 0; + /** + * ⚠️ What `e.target.value` can actually hold here — MEASURED in a real + * browser, not inferred from the spec (objectui#6765). + * + * The bare `parseFloat` below has no whole-string guard of its own, and that + * is deliberate rather than objectui#6715's defect repeated. This is a + * `type="number"` input, and a real browser never exposes non-numeric + * residue through `.value` — keystrokes and pastes are filtered before the + * change event fires. + * + * Measured on Chromium 141.0.7390.37 (Playwright 1.62.1), driving THIS + * widget (fraction convention, `precision: 2`): + * + * ``` + * typed "12abc" -> box.value "12" onChange(0.12) + * typed "1.2.3" -> box.value "1.23" onChange(0.0123) + * pasted "0x10" -> box.value "010" onChange(0.1) + * typed "1e" -> box.value "" onChange(null) validity.badInput + * ``` + * + * ⛔ objectui#6715's anchored `WHOLE_NUMBER_TEXT` is deliberately NOT copied + * here: it accepts every string this box can produce and rejects only + * strings the TEST environment fabricates, because happy-dom does not + * implement the sanitization. See the fuller note in `CurrencyField.tsx`, + * and the pinned oracle-vs-product table in + * `__tests__/NumberInputWidgets.environmentDivergence.test.tsx`. + * + * ⚠️ OPEN (objectui#6765): the last row is a SILENT drop — the box keeps + * DISPLAYING `1e` while `.value` reads `''`, so this emits `null` with + * `aria-invalid` still `false` and no diagnostic drawn (objectui#6716's + * class). Escalated, not fixed here — it is shared by every `type="number"` + * widget in this package. + */ const handleChange = (e: React.ChangeEvent) => { if (e.target.value === '') { onChange(null as any);