From ec938a22b74a8329b36c1d3656d17aa030205380 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 07:24:11 +0000 Subject: [PATCH] fix(fields): parse LocationField coordinates as strict whole-string numbers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bare `parseFloat` stops at the first character it cannot read and returns what it got, so each half of the typed pair was accepted as if it were whole: `"12abc, 34"` emitted `{ lat: 12, lng: 34 }` — a coordinate nobody typed. Unlike objectui#6714, the platform validator cannot be the oracle: every one of those truncations is a pair `valueSchemaFor({ type: 'location' })` accepts, so nothing downstream could ever object. Measured on b76ca6764 through a real ObjectForm, `dataSource.create` was handed `{"lat":12,"lng":34}` with `aria-invalid="false"` and no diagnostic. The class is wider than the card's three: `0x10` truncates to `0`, and `"12.5 N, 34 E"` drops the hemisphere. Each half is now tested against `parseFloat`'s own grammar, ANCHORED — not a stricter notion of a number invented in the widget — so every form that is a number today still is: negatives, a leading `+`, surrounding whitespace, exponent forms, and a bare decimal point on either side. The refusal is announced through objectui#6716's `refusalError` machinery and names the half it could not read; a third silent refusal would have re-opened the defect #6716 had just closed. Two boundaries drawn deliberately: text with no number at the front keeps the pre-existing format sentence, and `Infinity` carries no residue so it is still refused by #6714's range arm. Degree/hemisphere notation is not parsed, per the maintainer ruling of 2026-08-29. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .../6715-location-strict-numeric-parse.md | 55 +++ .../LocationField.strictNumeric.test.tsx | 411 ++++++++++++++++++ packages/fields/src/widgets/LocationField.tsx | 104 ++++- .../src/ObjectForm.locationResidue.test.tsx | 232 ++++++++++ 4 files changed, 797 insertions(+), 5 deletions(-) create mode 100644 .changeset/6715-location-strict-numeric-parse.md create mode 100644 packages/fields/src/__tests__/LocationField.strictNumeric.test.tsx create mode 100644 packages/plugin-form/src/ObjectForm.locationResidue.test.tsx diff --git a/.changeset/6715-location-strict-numeric-parse.md b/.changeset/6715-location-strict-numeric-parse.md new file mode 100644 index 000000000..92faca145 --- /dev/null +++ b/.changeset/6715-location-strict-numeric-parse.md @@ -0,0 +1,55 @@ +--- +'@object-ui/fields': patch +--- + +`LocationField` no longer invents a coordinate out of text that is only partly a +number (objectui#6715). + +Each half of the typed pair was read with a bare `parseFloat`, which stops at the +first character it cannot read and returns what it got. So `"12abc, 34"` emitted +`{ lat: 12, lng: 34 }` — a coordinate nobody typed. + +**Why nothing downstream could catch it, and why that makes this different from +objectui#6714.** Every one of those truncations is a pair +`valueSchemaFor({ type: 'location' })` ACCEPTS: well-formed, in range, and wrong. +#6714's `999, 999` was at least a value the contract refuses, so something +downstream could in principle have objected; here the platform validator cannot +be the oracle at all. Measured on `b76ca6764` by driving a real `ObjectForm` +(create mode, a `type: 'location'` field, a fake `DataSource`) and submitting: + +``` +typed "12abc, 34" create({ place: {"lat":12,"lng":34} }) aria-invalid=false +typed "1.2.3, 4" create({ place: {"lat":1.2,"lng":4} }) aria-invalid=false +typed "12deg, 34" create({ place: {"lat":12,"lng":34} }) aria-invalid=false +typed "0x10, 34" create({ place: {"lat":0,"lng":34} }) aria-invalid=false +typed "12.5 N, 34 E" create({ place: {"lat":12.5,"lng":34} }) aria-invalid=false +``` + +The last two show the size of the class. `0x10` truncates to `0` — objectui#6272's +`|| 0` in the Gulf of Guinea, arriving through a different door — and +`"12.5 N, 34 E"` drops the hemisphere, so a `12.5 S` paste would have been stored +as `+12.5`, on the wrong side of the equator, with nothing said. + +**The fix** parses each half as a strict whole-string number, applying +objectui#6272's precedent: a field that renders a plausible wrong place is worse +than one that renders nothing. The test is `parseFloat`'s OWN grammar, anchored — +not a stricter notion of a number invented in the widget — so every form that is +a number today still is: negatives, a leading `+`, surrounding whitespace, +exponent forms (`3.027e1`), and a bare decimal point on either side (`.5`, `30.`). + +The refusal is **announced**, through the machinery objectui#6716 landed rather +than a new one, and it names the half it could not read: *Not saved: latitude +"12abc" is not a number. Enter plain decimals (example: 30.2741, 120.1551).* A +third silent refusal would have re-opened the defect #6716 had just closed. + +Two boundaries drawn deliberately: + +- Text with **no** number at the front (`abc`, `NaN`, `here, there`) keeps the + pre-existing format sentence. "No number at all" and "a number with text after + it" are different mistakes and get different advice. +- `Infinity` carries no residue — `parseFloat` reads the whole word — so it is + still refused by objectui#6714's **range** arm, not by the new one. + +⛔ Degree/hemisphere notation (`12°N, 34°E`) is **not** parsed. It stays refused, +per the maintainer ruling of 2026-08-29: the paste route is unmeasured, and it +becomes its own feature card if real demand arrives. diff --git a/packages/fields/src/__tests__/LocationField.strictNumeric.test.tsx b/packages/fields/src/__tests__/LocationField.strictNumeric.test.tsx new file mode 100644 index 000000000..f8a329175 --- /dev/null +++ b/packages/fields/src/__tests__/LocationField.strictNumeric.test.tsx @@ -0,0 +1,411 @@ +/** + * 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#6715 — `LocationField` must not INVENT a coordinate out of text + * that is only partly a number. + * + * The defect: the widget read each half of the typed pair with a bare + * `parseFloat`, which stops at the first character it cannot read and returns + * what it got. So `"12abc, 34"` emitted `{ lat: 12, lng: 34 }` — a coordinate + * nobody typed. + * + * ## Why the platform validator cannot be the oracle here + * + * This is what separates the card from objectui#6714, and it is asserted + * rather than asserted-by-comment below: every one of those truncated + * emissions is a pair `valueSchemaFor({ type: 'location' })` ACCEPTS. #6714's + * `999, 999` was at least a value the contract refuses, so something + * downstream could in principle have caught it; a truncation is well-formed, + * in range, and WRONG. Nothing downstream can ever object, which is why the + * refusal has to happen at the point of typing. + * + * ## What was measured on the base commit (`b76ca6764`), before the fix + * + * Driving a real `ObjectForm` (create mode, a `type: 'location'` field, a fake + * `DataSource`) and typing each text, then submitting: + * + * ``` + * typed "12abc, 34" stored {"lat":12,"lng":34} aria-invalid=false + * typed "1.2.3, 4" stored {"lat":1.2,"lng":4} aria-invalid=false + * typed "12deg, 34" stored {"lat":12,"lng":34} aria-invalid=false + * typed "0x10, 34" stored {"lat":0,"lng":34} aria-invalid=false + * typed "12.5 N, 34 E" stored {"lat":12.5,"lng":34} aria-invalid=false + * ``` + * + * The last two are the ones that show the size of the class. `0x10` truncates + * to `0` — objectui#6272's `|| 0` in the Gulf of Guinea, arriving through a + * different door — and `"12.5 N, 34 E"`, the PASTE shape triage guessed the + * real user route to be, drops the hemisphere, so a southern coordinate would + * be stored as a northern one. That end-to-end reading is pinned in + * `packages/plugin-form/src/ObjectForm.locationResidue.test.tsx`. + * + * ## The ruling this implements, and its fence + * + * The maintainer ruling of 2026-08-29 adopts REFUSAL: text carrying + * non-numeric residue is a non-coordinate and is refused loudly, never + * silently truncated. It applies objectui#6272's precedent — "a field that + * renders a plausible wrong place is worse than one that renders nothing". + * + * ⛔ Degree/hemisphere notation parsing (`12°N, 34°E`) is deliberately NOT + * ruled and NOT built: the paste route is unmeasured, and it becomes its own + * feature card if real demand arrives. The last describe block is the pin that + * it was not smuggled in. + * + * ## Where the refusal is announced + * + * Through the SAME `refusalError` machinery objectui#6716 landed minutes + * earlier, not a new one — that sequencing is why this card was held. A third + * silent refusal would have re-opened the defect #6716 had just closed, on a + * third input class. The pre-existing arms are pinned here as UNDISTURBED. + */ + +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen, fireEvent, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { valueSchemaFor } from '@objectstack/spec/data'; + +import { LocationField, type LocationValue } from '../widgets/LocationField'; + +const LOCATION_SCHEMA = valueSchemaFor({ type: 'location' } as any)!; + +const field = { name: 'site', label: 'Site', type: 'location' } as any; + +/** + * A stored, in-range value — the "prior value" a refusal must leave standing. + * + * ⚠️ It must differ from every coordinate typed below: the box is a CONTROLLED + * input, so `fireEvent.change` with text equal to what is already displayed + * fires no change event at all. + */ +const STORED: LocationValue = { lat: 10, lng: 20 }; + +function box(): HTMLInputElement { + return screen.getByRole('textbox') as HTMLInputElement; +} + +/** The widget's own refusal line, or `null` when it is announcing nothing. */ +function diagnostic(container: HTMLElement): string | null { + const p = container.querySelector('p'); + return p ? p.textContent : null; +} + +function typeInto(text: string, value: unknown = STORED) { + cleanup(); + const onChange = vi.fn(); + const { container } = render( + , + ); + fireEvent.change(box(), { target: { value: text } }); + return { + container, + emissions: onChange.mock.calls.map(c => c[0]), + ariaInvalid: box().getAttribute('aria-invalid'), + domValue: box().value, + message: diagnostic(container), + }; +} + +/** The exact sentence the pre-existing FORMAT arm draws (objectui#6716). */ +const FORMAT_MESSAGE = 'Not saved: enter a latitude, longitude pair (example: 30.2741, 120.1551).'; + +/** + * What the SPEC says about a pair, rendered the way the RANGE arm renders it. + * + * ⛔ Never typed out with `90` / `180` in it — a bound copied into this file is + * a second contract that keeps passing on the day the schema moves. + */ +function expectedRangeMessage(pair: unknown): string { + const parsed = LOCATION_SCHEMA.safeParse(pair); + if (parsed.success) throw new Error('expectedRangeMessage called on a pair the spec ACCEPTS'); + const detail = parsed.error.issues + .map((i: any) => `${i.path.join('.') || 'value'}: ${i.message}`) + .join('; '); + return `Not saved: ${detail}`; +} + +/** What a bare `parseFloat` WOULD have made of this text — the old reading. */ +function truncationOf(text: string): { lat: number; lng: number } { + const parts = text.split(',').map(p => p.trim()); + return { lat: parseFloat(parts[0]), lng: parseFloat(parts[1]) }; +} + +/* -------------------------------------------------------------------------- */ +/* 1. The card's own three: residue is refused, and announced. */ +/* -------------------------------------------------------------------------- */ + +describe('LocationField refuses a partly-numeric coordinate (objectui#6715)', () => { + it.each([ + ['12abc, 34', 'latitude', '12abc'], + ['1.2.3, 4', 'latitude', '1.2.3'], + ['12deg, 34', 'latitude', '12deg'], + ])('%s emits nothing and says which half it could not read', (typed, label, half) => { + // The premise the whole card rests on, asserted rather than assumed: the + // value the OLD reading would have emitted is one the platform ACCEPTS, so + // no downstream check could ever have caught it. + expect(LOCATION_SCHEMA.safeParse(truncationOf(typed)).success).toBe(true); + + const r = typeInto(typed); + expect(r.emissions).toEqual([]); + expect(r.ariaInvalid).toBe('true'); + expect(r.message).toContain(`${label} "${half}"`); + expect(r.message).toContain('Not saved:'); + // The refused text stays in the box, so the message has something to point + // at — objectui#6716's shape, inherited rather than reinvented. + expect(r.domValue).toBe(typed); + }); + + it('names BOTH halves when both carry residue', () => { + const r = typeInto('12abc, 34xyz'); + expect(r.emissions).toEqual([]); + expect(r.message).toBe( + 'Not saved: latitude "12abc" and longitude "34xyz" are not numbers. ' + + 'Enter plain decimals (example: 30.2741, 120.1551).', + ); + }); + + it('names only the offending half when the other one is fine', () => { + const r = typeInto('30.27, 120abc'); + expect(r.message).toBe( + 'Not saved: longitude "120abc" is not a number. Enter plain decimals (example: 30.2741, 120.1551).', + ); + expect(r.message).not.toContain('latitude'); + }); + + it('does not disturb the value already stored', () => { + const r = typeInto('12abc, 34'); + expect(r.emissions).toEqual([]); + // Nothing was emitted, so nothing replaced STORED — the same rule the + // format and range arms follow. + expect(LOCATION_SCHEMA.safeParse(STORED).success).toBe(true); + }); + + it('clears the announcement once the residue is corrected, and then emits', () => { + cleanup(); + const onChange = vi.fn(); + const { container } = render( + , + ); + fireEvent.change(box(), { target: { value: '12abc, 34' } }); + expect(box()).toHaveAttribute('aria-invalid', 'true'); + expect(diagnostic(container)).not.toBeNull(); + + fireEvent.change(box(), { target: { value: '30.2741, 120.1551' } }); + expect(box()).toHaveAttribute('aria-invalid', 'false'); + expect(diagnostic(container)).toBeNull(); + expect(onChange.mock.calls.map(c => c[0])).toEqual([{ lat: 30.2741, lng: 120.1551 }]); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* 2. The residue class is wider than the three the card names. */ +/* -------------------------------------------------------------------------- */ + +describe('LocationField refuses every truncation, not just the obvious junk (objectui#6715)', () => { + it.each([ + // A hex literal: `parseFloat('0x10')` is 0 — objectui#6272's `|| 0`, and a + // real place in the Gulf of Guinea, reached through a different door. + ['0x10, 34'], + ['0b11, 34'], + // Digit-grouping and a half-typed exponent. + ['1_000, 34'], + ['1e, 2'], + // Two numbers where one was expected. + ['12 34, 56'], + // The PASTE shapes triage guessed the real user route to be. Today these + // silently DROP the hemisphere — `12.5 S` would store as `+12.5`. + ['12deg, 34deg'], + ['12.5 N, 34 E'], + ])('%s is refused instead of truncated', typed => { + // Same premise as above: the old reading produced a pair the spec accepts. + const wouldHaveBeen = truncationOf(typed); + expect(Number.isNaN(wouldHaveBeen.lat) || Number.isNaN(wouldHaveBeen.lng)).toBe(false); + expect(LOCATION_SCHEMA.safeParse(wouldHaveBeen).success).toBe(true); + + const r = typeInto(typed); + expect(r.emissions).toEqual([]); + expect(r.ariaInvalid).toBe('true'); + expect(r.message).toContain('Not saved:'); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* 3. The other direction — the boundary, stated explicitly. */ +/* -------------------------------------------------------------------------- */ + +describe('LocationField still accepts every WHOLE-STRING number (objectui#6715)', () => { + it.each([ + ['30.27, 120.15', { lat: 30.27, lng: 120.15 }], + // The pin the card asks for by name: this is what catches an over-strict + // parse, and it is the emission that must be byte-for-byte what it is today. + ['30.2741, 120.1551', { lat: 30.2741, lng: 120.1551 }], + // Negatives — the southern and western hemispheres. + ['-30.27, -120.15', { lat: -30.27, lng: -120.15 }], + // A leading `+`, which `parseFloat` reads today. + ['+30.27, +120.15', { lat: 30.27, lng: 120.15 }], + // Leading and trailing whitespace, around each half and around the pair. + [' 30.27 , 120.15 ', { lat: 30.27, lng: 120.15 }], + // Exponent form, in both spellings, since `parseFloat` accepts it today. + ['3.027e1, 1.2015e2', { lat: 30.27, lng: 120.15 }], + ['3.027E1, -1.2015E2', { lat: 30.27, lng: -120.15 }], + ['1.5e-3, 4', { lat: 0.0015, lng: 4 }], + // A bare decimal point on either side of the digits — both are numbers + // JS reads whole, and both occur mid-typing. + ['.5, .25', { lat: 0.5, lng: 0.25 }], + ['-.5, +.25', { lat: -0.5, lng: 0.25 }], + ['30., 120.', { lat: 30, lng: 120 }], + // Integers, the plainest form of all. + ['0, 0', { lat: 0, lng: 0 }], + ['90, 180', { lat: 90, lng: 180 }], + ['-90, -180', { lat: -90, lng: -180 }], + ])('%s still emits exactly what it emits today', (typed, expected) => { + const r = typeInto(typed, null); + expect(r.emissions).toEqual([expected]); + expect(r.message).toBeNull(); + expect(r.ariaInvalid).toBe('false'); + }); + + /** + * The property behind that table, so a future widening of the grammar cannot + * pass by adding a row: for every half this widget calls a number, JS's own + * whole-string reading (`Number`) must agree with `parseFloat`'s. + * + * That is the ONE-DIRECTIONAL check that matters here — it fails the moment + * the gate starts accepting residue (`Number('12abc')` is `NaN`, while + * `parseFloat('12abc')` is `12`). It is deliberately not the definition: + * `Number` also reads `'0x10'` as `16` and `''` as `0`, which is why the + * widget does not use it as the test. + */ + it('every accepted half is a number JS reads whole', () => { + // Every half is within the LATITUDE range, so the only thing that can + // refuse it is the numeric gate under test — not objectui#6714's range arm. + const accepted = [ + '30.27', '-30.27', '+30.27', '3.027e1', '8.5E1', '1.5e-3', + '.5', '-.5', '+.25', '30.', '0', '90', '-90', + ]; + for (const half of accepted) { + const r = typeInto(`${half}, 0`, null); + expect(r.emissions.length, `half "${half}" was refused`).toBe(1); + expect(Number(half), `half "${half}"`).toBe(parseFloat(half)); + } + }); + + it('clearing the box still emits null', () => { + const r = typeInto(''); + expect(r.emissions).toEqual([null]); + expect(r.message).toBeNull(); + }); + + it('still carries the optional keys across a clean edit (objectui#6664)', () => { + const r = typeInto('31.2304, 121.4737', { lat: 30.2741, lng: 120.1551, altitude: 5, accuracy: 12 }); + expect(r.emissions).toEqual([{ lat: 31.2304, lng: 121.4737, altitude: 5, accuracy: 12 }]); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* 4. The objectui#6716 arms are NOT disturbed. */ +/* -------------------------------------------------------------------------- */ + +describe('LocationField leaves the pre-existing refusal arms exactly as they were (objectui#6716)', () => { + it('text with no number in it at all still gets the FORMAT sentence', () => { + // `parseFloat` reads NOTHING here, so this never reaches the new gate. The + // split is deliberate: "no number at the front" and "a number with text + // after it" are different mistakes and get different advice. + for (const typed of ['not a coordinate', 'here, there', '--1, 2', '(12), 34']) { + const r = typeInto(typed); + expect(r.emissions, typed).toEqual([]); + expect(r.message, typed).toBe(FORMAT_MESSAGE); + expect(r.ariaInvalid, typed).toBe('true'); + } + }); + + it('a non-pair still gets the FORMAT sentence', () => { + for (const typed of ['30.27', '30.27, 120.15, 5']) { + const r = typeInto(typed); + expect(r.emissions, typed).toEqual([]); + expect(r.message, typed).toBe(FORMAT_MESSAGE); + } + }); + + it.each([ + ['999, 999', { lat: 999, lng: 999 }], + ['91, 0', { lat: 91, lng: 0 }], + ['0, 181', { lat: 0, lng: 181 }], + ['-91, 0', { lat: -91, lng: 0 }], + ['0, -181', { lat: 0, lng: -181 }], + ])('%s still gets the RANGE sentence, built from the spec', (typed, pair) => { + const r = typeInto(typed); + expect(r.emissions).toEqual([]); + expect(r.message).toBe(expectedRangeMessage(pair)); + }); + + /** + * The one boundary decision inside this card that could have moved an + * existing arm, made deliberately and pinned so it cannot drift. + * + * `Infinity` carries NO residue — `parseFloat` reads the whole word — so the + * new gate has nothing to say about it and it goes on to the RANGE arm, + * which is where objectui#6714 put it and what that card's docblock in + * `LocationField.tsx` still describes. A strictness that swallowed it here + * would have silently invalidated a minutes-old explanation while keeping + * every "no emission" pin green. + */ + it('an infinite coordinate is still refused by the RANGE arm, not the new one', () => { + const r = typeInto('Infinity, 0'); + expect(r.emissions).toEqual([]); + expect(r.message).toBe(expectedRangeMessage({ lat: Infinity, lng: 0 })); + expect(r.message).not.toContain('is not a number'); + }); + + it('a HOST-produced error is still the host\'s alone (objectui#3222)', () => { + cleanup(); + const { container } = render( + , + ); + expect(box()).toHaveAttribute('aria-invalid', 'true'); + expect(container.textContent).not.toContain('Host says this field is required'); + expect(diagnostic(container)).toBeNull(); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* 5. The ruling's fence: no degree/hemisphere parsing was smuggled in. */ +/* -------------------------------------------------------------------------- */ + +describe('LocationField does NOT parse degree or hemisphere notation (objectui#6715 ruling)', () => { + it.each([ + ['12°N, 34°E'], + ['12.5 N, 34 E'], + ['12° 30\' N, 34° 15\' E'], + ['12 N, 34 E'], + ])('%s is refused, not converted', typed => { + const r = typeInto(typed); + // Nothing emitted: the notation was not learned, which is the ruling. + expect(r.emissions).toEqual([]); + expect(r.ariaInvalid).toBe('true'); + }); + + it('advises plain decimals rather than the notation it refuses', () => { + const r = typeInto('12\u00b0N, 34\u00b0E'); + // Nothing emitted: the notation was not learned, which is the ruling. + expect(r.emissions).toEqual([]); + // The ADVICE names the form this widget does read. Degree symbols do appear + // in the sentence, but only as the echo of what was typed — the guidance + // itself must never point at a route this widget refuses, or it would + // promise the very parse the ruling declined. + expect(r.message).toContain('Enter plain decimals (example: 30.2741, 120.1551).'); + const advice = r.message!.slice(r.message!.indexOf('Enter plain decimals')); + expect(advice).not.toContain('\u00b0'); + }); +}); diff --git a/packages/fields/src/widgets/LocationField.tsx b/packages/fields/src/widgets/LocationField.tsx index d078d1535..3bf3a6172 100644 --- a/packages/fields/src/widgets/LocationField.tsx +++ b/packages/fields/src/widgets/LocationField.tsx @@ -143,18 +143,69 @@ function coordinateText(value: unknown): string { return isLocationValue(value) ? `${value.lat}, ${value.lng}` : ''; } +/** + * The one place this widget says what "a number" is (objectui#6715). + * + * ⚠️ This is `parseFloat`'s OWN grammar, ANCHORED — not a second, stricter + * notion of a number invented here. `parseFloat` reads the longest PREFIX of + * its argument matching this grammar and returns what it got, discarding the + * rest; the anchors are what turn "there is a number at the front" into "the + * whole text IS that number". Nothing else about the reading changes, which is + * why {@link parseDraft} still asks `parseFloat` for the value itself. + * + * The defect the anchors exist for: `parseFloat('12abc')` is `12`, so + * `"12abc, 34"` emitted `{ lat: 12, lng: 34 }` — a coordinate the user never + * typed, which `valueSchemaFor({ type: 'location' })` ACCEPTS, so unlike + * objectui#6714 no downstream check could ever catch it. Measured on + * `b76ca6764` through a real `ObjectForm`: `dataSource.create` was handed + * `{"lat":12,"lng":34}` with `aria-invalid="false"` and no diagnostic drawn. + * Truncation is not confined to obvious junk, either — the same reading turns + * `"0x10"` into `0` (objectui#6272's `|| 0`, arriving through a different + * door) and `"12.5 N, 34 E"` into `{ lat: 12.5, lng: 34 }`, dropping the + * hemisphere so a southern coordinate would be stored as a northern one. + * + * ⛔ `Number()` is NOT this test, although it looks like the same idea: it + * reads `'0x10'` as `16`, `'0b11'` as `3` and `''` as `0`. A hex literal is + * not a coordinate notation, and none of those readings is what was typed. + * + * ⛔ Nor is this degree/hemisphere PARSING. `12°N` stays refused, deliberately + * — the maintainer ruling of 2026-08-29 adopts the refusal and declines the + * notation, because the paste route is unmeasured; it is its own feature card + * if real demand arrives. + * + * `Infinity` IS in the grammar, deliberately. `parseFloat` reads it whole, so + * it carries no residue and this gate has nothing to say about it; it is + * refused one step later by {@link isSpecAcceptedLocation}, exactly as it is + * today (objectui#6714). The range arm keeps its own case rather than having + * it quietly moved into this one. + */ +const WHOLE_NUMBER_TEXT = /^[+-]?(?:Infinity|(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?)$/; + +/** The two coordinates, in the order they are typed, named for the diagnostic. */ +const COORDINATE_LABELS = ['latitude', 'longitude'] as const; + +/** One half of the typed pair that carried non-numeric residue. */ +type ResidueHalf = { label: string; text: string }; + /** * What the typed text means to this widget, as ONE reading (objectui#6716). * - * The three outcomes are exactly the three the emission rule already had — - * cleared / not a coordinate pair / a pair — lifted out of `handleChange` so - * the DRAFT-SYNC guard below judges the text by the same rule that decides - * whether to emit it. Two copies of "is this a coordinate pair" would be two + * The outcomes are exactly the ones the emission rule already had — cleared / + * not a coordinate pair / a pair — lifted out of `handleChange` so the + * DRAFT-SYNC guard below judges the text by the same rule that decides whether + * to emit it. Two copies of "is this a coordinate pair" would be two * contracts, and the one in the effect would be the one nobody tests. + * + * objectui#6715 adds a fourth, `residue`, splitting what used to be one + * reading of "the half is a number" into the two different things `parseFloat` + * was conflating: no number at the front at all (still `unparsable`, still the + * pre-existing FORMAT arm) versus a number at the front with text after it. + * Only the second is new; the first keeps its message and its #6716 pins. */ type ParsedDraft = | { kind: 'cleared' } | { kind: 'unparsable' } + | { kind: 'residue'; residue: ResidueHalf[] } | { kind: 'pair'; lat: number; lng: number }; function parseDraft(text: string): ParsedDraft { @@ -163,7 +214,15 @@ function parseDraft(text: string): ParsedDraft { if (parts.length !== 2) return { kind: 'unparsable' }; const lat = parseFloat(parts[0]); const lng = parseFloat(parts[1]); + // No number at the front of a half AT ALL (`abc`, `NaN`, `--1`): the + // pre-existing FORMAT arm, deliberately left where it was so its sentence + // and its objectui#6716 pins keep saying exactly what they said. if (isNaN(lat) || isNaN(lng)) return { kind: 'unparsable' }; + // A number at the front, but not all the way to the end (objectui#6715). + const residue = COORDINATE_LABELS + .map((label, i): ResidueHalf => ({ label, text: parts[i] })) + .filter(half => !WHOLE_NUMBER_TEXT.test(half.text)); + if (residue.length > 0) return { kind: 'residue', residue }; return { kind: 'pair', lat, lng }; } @@ -179,7 +238,10 @@ function parseDraft(text: string): ParsedDraft { function draftDenotes(text: string, value: unknown): boolean { const parsed = parseDraft(text); if (parsed.kind === 'cleared') return !isLocationValue(value); - if (parsed.kind === 'unparsable') return false; + // Text this widget REFUSES denotes no stored value — `unparsable`, and since + // objectui#6715 `residue` too. Written as "not a pair" rather than as a list + // of refusal kinds, so a future arm cannot be forgotten here. + if (parsed.kind !== 'pair') return false; return isLocationValue(value) && value.lat === parsed.lat && value.lng === parsed.lng; } @@ -217,6 +279,28 @@ function refusedRangeMessage(candidate: LocationValue): string { return `Not saved: ${detail}`; } +/** + * What the box says when a half of the pair is only PARTLY a number + * (objectui#6715). + * + * ⛔ Deliberately NOT {@link REFUSED_FORMAT_MESSAGE}. "Enter a latitude, + * longitude pair" is unusable advice to someone who typed `12abc, 34`: they + * DID type a pair, and that sentence gives them nothing to correct. This + * refusal names the half that could not be read and quotes it back, because + * the residue IS the content of this refusal — the same principle by which the + * format arm names the format and the range arm reports the spec's own + * complaint. + * + * ⛔ It does not suggest a notation to convert FROM (no `12°N` advice): the + * ruling declines that parse, so pointing at it would advertise a route this + * widget refuses. + */ +function refusedResidueMessage(residue: readonly ResidueHalf[]): string { + const named = residue.map(half => `${half.label} "${half.text}"`).join(' and '); + const verb = residue.length > 1 ? 'are not numbers' : 'is not a number'; + return `Not saved: ${named} ${verb}. Enter plain decimals (example: 30.2741, 120.1551).`; +} + /** * LocationField - Geographic coordinate input for a `type: 'location'` value. * @@ -332,6 +416,16 @@ export function LocationField({ value, onChange, field, readonly, error, ...prop return; } + if (parsed.kind === 'residue') { + // objectui#6715: a half that is only PARTLY a number is a NON-COORDINATE, + // not a number to truncate. Refused, and announced through the very same + // `setRefusalError` the other two arms use — a third SILENT refusal is + // precisely the defect objectui#6716 had just finished removing, which is + // why this card was held until #6716 landed. + setRefusalError(refusedResidueMessage(parsed.residue)); + return; + } + // The typed pair replaces `lat`/`lng`; `altitude`/`accuracy` survive the // edit (objectui#6664). Key-by-key, never a spread — see above. const emitted = carryOptionalKeys(parsed.lat, parsed.lng, value); diff --git a/packages/plugin-form/src/ObjectForm.locationResidue.test.tsx b/packages/plugin-form/src/ObjectForm.locationResidue.test.tsx new file mode 100644 index 000000000..8c5f96d2c --- /dev/null +++ b/packages/plugin-form/src/ObjectForm.locationResidue.test.tsx @@ -0,0 +1,232 @@ +/** + * 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#6715, measured end to end: a partly-numeric coordinate must never + * reach storage as a whole one. + * + * The two sibling files are the other halves of the same story — + * `ObjectForm.locationRange.test.tsx` is objectui#6714's measurement (an + * out-of-range pair never reaches `dataSource.create`) and + * `ObjectForm.locationRefusal.test.tsx` is objectui#6716's (a refusal is + * announced rather than swallowed). This file is the class NEITHER of them + * covers, and the reason it needed its own card: a TRUNCATED pair. + * + * ## Why this file exists rather than a widget-only pin + * + * The defect is invisible to every oracle except the user's own typing. + * `parseFloat('12abc')` is `12`, so `"12abc, 34"` produced + * `{ lat: 12, lng: 34 }` — well-formed, in range, and something + * `valueSchemaFor({ type: 'location' })` ACCEPTS. #6714's `999, 999` was at + * least a value the contract refuses; a truncation is a value the contract + * blesses. So the only place the defect is observable is HERE, at what the + * form actually hands to `dataSource.create`. Each case below asserts that + * premise from the spec before asserting the refusal. + * + * ## What this measured on the base commit (`b76ca6764`), before the fix + * + * Typing each text into this same form and submitting: + * + * ``` + * typed "12abc, 34" create({ place: {"lat":12,"lng":34} }) aria-invalid=false + * typed "1.2.3, 4" create({ place: {"lat":1.2,"lng":4} }) aria-invalid=false + * typed "12deg, 34" create({ place: {"lat":12,"lng":34} }) aria-invalid=false + * typed "0x10, 34" create({ place: {"lat":0,"lng":34} }) aria-invalid=false + * typed "12.5 N, 34 E" create({ place: {"lat":12.5,"lng":34} }) aria-invalid=false + * ``` + * + * The row for `"12.5 N, 34 E"` is the one worth reading twice: that is the + * PASTE shape triage guessed the real user route to be, and the hemisphere + * letter is silently dropped — a `12.5 S` paste would have been stored as + * `+12.5`, on the wrong side of the equator, with nothing said. + * + * ## Scope, from the ruling + * + * ⛔ Degree/hemisphere notation is NOT parsed — the ruling declines it because + * the paste route is unmeasured. The refusal is the whole change. + * + * ⛔ `buildValidationRules` still has NO `location` branch, and this card did + * not give it one — the last test is the pin that keeps + * `ObjectForm.locationRange.test.tsx`'s docblock true. It could not have one: + * a refusal means `onChange` never fires, so the typed text never becomes a + * form value for a value-shaped rule to see (measured on objectui#6716). + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, waitFor, fireEvent } from '@testing-library/react'; +import React from 'react'; +import { valueSchemaFor } from '@objectstack/spec/data'; + +import { ObjectForm } from './ObjectForm'; +import { registerAllFields, buildValidationRules } from '@object-ui/fields'; + +registerAllFields(); + +const LOCATION_SCHEMA = valueSchemaFor({ type: 'location' } as any)!; + +const siteSchema = { + name: 'site', + fields: { + title: { type: 'text', label: 'Title' }, + place: { type: 'location', label: 'Place' }, + }, +}; + +const makeDS = () => ({ + getObjectSchema: vi.fn().mockResolvedValue(siteSchema), + create: vi.fn(async (_o: string, d: any) => ({ id: 'r1', ...d })), + update: vi.fn(), + findOne: vi.fn(), +}); + +const waitInput = (c: HTMLElement, name: string) => + waitFor(() => { + const el = c.querySelector(`input[name="${name}"]`) as HTMLInputElement | null; + if (!el) throw new Error(`${name} not ready`); + return el; + }); + +/** The whole rendered row for `place`, so any text the field draws is seen. */ +function placeRowText(container: HTMLElement): string { + let node: HTMLElement | null = container.querySelector('input[name="place"]'); + for (let i = 0; i < 4 && node?.parentElement; i++) node = node.parentElement; + return (node?.textContent || '').trim(); +} + +function mountForm() { + const ds = makeDS(); + const { container } = render( + , + ); + return { ds, container }; +} + +/** Type a coordinate, submit, and report what the form actually did. */ +async function typeAndSubmit(typed: string) { + const { ds, container } = mountForm(); + fireEvent.change(await waitInput(container, 'title'), { target: { value: 'HQ' } }); + fireEvent.change(await waitInput(container, 'place'), { target: { value: typed } }); + + const el = container.querySelector('input[name="place"]') as HTMLInputElement; + const observed = { + ariaInvalid: el.getAttribute('aria-invalid'), + domValue: el.value, + rowText: placeRowText(container), + }; + + fireEvent.submit(container.querySelector('form') as HTMLFormElement); + await waitFor(() => expect(ds.create).toHaveBeenCalled()); + return { ...observed, stored: ds.create.mock.calls[0][1].place }; +} + +/** What a bare `parseFloat` WOULD have stored — the reading this card removes. */ +function truncationOf(text: string): { lat: number; lng: number } { + const parts = text.split(',').map(p => p.trim()); + return { lat: parseFloat(parts[0]), lng: parseFloat(parts[1]) }; +} + +/* -------------------------------------------------------------------------- */ +/* The defect, at the only place it is observable. */ +/* -------------------------------------------------------------------------- */ + +describe('ObjectForm never stores a truncated coordinate (objectui#6715)', () => { + it.each([ + ['12abc, 34'], + ['1.2.3, 4'], + ['12deg, 34'], + // The wider class, all of which stored a plausible wrong place on the base + // commit: a hex literal reading as 0, and the paste shapes that drop the + // hemisphere. + ['0x10, 34'], + ['12.5 N, 34 E'], + ['12deg, 34deg'], + ])('%s is refused, announced, and never reaches dataSource.create', async typed => { + // 1. The premise: the OLD reading produced a pair the platform ACCEPTS, so + // nothing downstream could have objected to it. This is what makes the + // card different from objectui#6714. + const wouldHaveBeen = truncationOf(typed); + expect(Number.isNaN(wouldHaveBeen.lat)).toBe(false); + expect(LOCATION_SCHEMA.safeParse(wouldHaveBeen).success).toBe(true); + + const after = await typeAndSubmit(typed); + + // 2. Nothing was stored — not the truncation, not anything. + expect(after.stored).toBeUndefined(); + // 3. The refusal is ANNOUNCED, through objectui#6716's machinery: the a11y + // state a screen reader reads, and a reason a person can read. + expect(after.ariaInvalid).toBe('true'); + expect(after.rowText).toContain('Not saved:'); + // 4. The refused text is still in the box to be corrected. + expect(after.domValue).toBe(typed); + }); + + it('names the half it could not read, so the message is correctable', async () => { + const after = await typeAndSubmit('12abc, 34'); + expect(after.rowText).toContain('latitude "12abc"'); + }); + + it('stores the coordinate once the residue is corrected', async () => { + const { ds, container } = mountForm(); + fireEvent.change(await waitInput(container, 'title'), { target: { value: 'HQ' } }); + fireEvent.change(await waitInput(container, 'place'), { target: { value: '12abc, 34' } }); + expect( + (container.querySelector('input[name="place"]') as HTMLInputElement).getAttribute('aria-invalid'), + ).toBe('true'); + + fireEvent.change(await waitInput(container, 'place'), { target: { value: '30.2741, 120.1551' } }); + const el = container.querySelector('input[name="place"]') as HTMLInputElement; + expect(el.getAttribute('aria-invalid')).toBe('false'); + expect(placeRowText(container)).not.toContain('Not saved:'); + + fireEvent.submit(container.querySelector('form') as HTMLFormElement); + await waitFor(() => expect(ds.create).toHaveBeenCalled()); + const stored = ds.create.mock.calls[0][1].place; + expect(stored).toEqual({ lat: 30.2741, lng: 120.1551 }); + expect(LOCATION_SCHEMA.safeParse(stored).success).toBe(true); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* The other direction: a clean pair costs nothing. */ +/* -------------------------------------------------------------------------- */ + +describe('ObjectForm still stores every clean numeric pair (objectui#6715)', () => { + it.each([ + ['30.27, 120.15', { lat: 30.27, lng: 120.15 }], + ['-30.27, -120.15', { lat: -30.27, lng: -120.15 }], + ['+30.27, +120.15', { lat: 30.27, lng: 120.15 }], + [' 30.27 , 120.15 ', { lat: 30.27, lng: 120.15 }], + ['3.027e1, 1.2015e2', { lat: 30.27, lng: 120.15 }], + ])('%s reaches storage unchanged', async (typed, expected) => { + const after = await typeAndSubmit(typed); + expect(after.stored).toEqual(expected); + expect(after.ariaInvalid).toBe('false'); + expect(after.rowText).not.toContain('Not saved:'); + expect(LOCATION_SCHEMA.safeParse(after.stored).success).toBe(true); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* The scope fence objectui#6744 owns. */ +/* -------------------------------------------------------------------------- */ + +describe('objectui#6715 did not give buildValidationRules a `location` branch', () => { + it('compiles no rules for a location field', () => { + // The fact `ObjectForm.locationRange.test.tsx`'s docblock asserts, kept + // true here as well. objectui#6744 owns the question of whether it should + // have one; this card did not answer it. + // It compiles rules key by key and returns `undefined` when none applied, + // so `undefined` here means "no branch matched a location field". + expect(buildValidationRules({ type: 'location', name: 'place' })).toBeUndefined(); + // A control from the same call, so the reading above cannot be an artefact + // of a helper that returns `undefined` for everything. + expect(buildValidationRules({ type: 'text', name: 'title', required: true })).toEqual({ required: true }); + }); +});