diff --git a/docs/working/plans/2026-07-23-post-form-location-override.md b/docs/working/plans/2026-07-23-post-form-location-override.md index f4edd81..dc88d67 100644 --- a/docs/working/plans/2026-07-23-post-form-location-override.md +++ b/docs/working/plans/2026-07-23-post-form-location-override.md @@ -11,7 +11,7 @@ execution: code # Post Form Location Override - Plan -**Status:** ✅ Complete (2026-07-24) +**Status:** ✅ Complete (2026-07-24) — U1–U6 shipped, then hardened by a multi-agent code review the same day. The review found the design's stated server-side safety net (`cleanCoordinate()`) had never been committed, so it landed here; replaced a prefix-parsing coordinate check that accepted `48abc` / `48,85` / `35.0116S` (hemisphere silently flipped); closed three paths that bypassed the submit gate (draft restore, edit-mode prefill, map-load failure) because the gate read a CSS class no code set at init; added pin removal on blanked fields; made the geocode failure visible; and rewrote the U5 guard spec, which asserted only instantly-passing conditions and so could not fail. R8 and R13 above are revised accordingly. **Still open before merge:** the `user/` submodule commit is unpushed by choice (git-sync would deploy to prod), so the pin must be pushed and re-pointed at merge time; and the Playwright suite has never executed end-to-end (`make test-account` is broken by a shell-special value in `.env`), so every verification below is by inspection, not by a green run. ## Goal Capsule @@ -45,7 +45,7 @@ The only way to set a coordinate today is the GPS button (reads live position) o - R5. Lookup is explicit-click only. While in flight, the button shows a disabled "Searching…" state that always re-enables on response, no-match, or network failure. - R6. Clicking with both City and Country empty is treated as a no-match: an inline hint asks for a city or country first, and no request is sent. - R7. Multiple matches render as a clickable list (place name, admin region, country), built via `document.createElement` + `.textContent` (no `innerHTML`), matching every other dynamic-content construction already in `post-form.js`. Clicking an entry sets `lat`/`lng` and the pin only — it never writes back to City/Country. The list hides again until the next lookup. -- R8. No matches renders an inline hint suggesting a country or manual pin drag; a network failure degrades silently (fields untouched), consistent with the existing reverse-geocode/weather error handling in `post-form.js`. +- R8. No matches renders an inline hint suggesting a country or manual pin drag; a network failure (or a non-2xx response) leaves the fields untouched and renders a *distinct* inline hint naming the connection as the problem. **Revised in code review 2026-07-24** from "degrades silently" — silence was indistinguishable from a broken button, and the two failure modes need different messages. **Map preview & sync** - R9. A single MapLibre GL map with one draggable marker (≥44×44px touch target) renders in the panel, reusing the site's existing style URL (`MAP_STYLE`, extracted to a shared `user/themes/intotheeast/js/src/map-style.js` module per KTD1). The map instance is created once, on the panel's first open, held in module scope, and reused (with an explicit `.resize()` call) on every subsequent open — the container sits under `display:none` while closed, so the first paint would otherwise get a zero-size canvas. @@ -54,7 +54,7 @@ The only way to set a coordinate today is the GPS button (reads live position) o - R12. No pin is shown until one of the four paths above sets a value for the first time. **Error handling & validation boundary** -- R13. Invalid manual `lat`/`lng` text is never client-blocked — the visual mismatch flag (R11) is the only feedback. Final enforcement stays server-side in `cleanCoordinate()`, which already throws on a non-blank, still-invalid value after cleaning. +- R13. Invalid manual `lat`/`lng` text raises the visual mismatch flag (R11), **and** an unresolved flag blocks submit. **Revised in code review 2026-07-24** from "never client-blocked". The original wording deferred all enforcement to a server-side `cleanCoordinate()` described as already shipped — it was not committed anywhere, so no layer validated coordinates. It now ships in `cache-on-save.php` (both the `/post` form and the Admin2/API save paths) and the client gate stays, giving real defence in depth. The client parse is intentionally stricter than the server's `is_numeric` (whole-value decimals only, so `48,85` / `35.0116S` / `48abc` are rejected rather than prefix-parsed). - R14. Geolocation permission denial keeps its existing, unmodified `#location-status` error behavior. ### Scope Boundaries diff --git a/docs/working/specs/2026-07-23-post-form-location-override-design.md b/docs/working/specs/2026-07-23-post-form-location-override-design.md index 7e90a9d..36d8fcf 100644 --- a/docs/working/specs/2026-07-23-post-form-location-override-design.md +++ b/docs/working/specs/2026-07-23-post-form-location-override-design.md @@ -64,8 +64,8 @@ Backend sanitization has already been added (`user/plugins/cache-on-save/cache-o ### Error handling - No search results: inline message under the search box, map/pin untouched. -- Search network failure: silent-ish degrade (consistent with existing weather/reverse-geocode error handling in `post-form.js`), fields untouched. -- Invalid manual `lat`/`lng` text: no client-side hard block (the map preview and eventual server-side `cleanCoordinate()` are the safety nets); this UI's whole point is to make that failure mode rare in practice, not to duplicate the backend validator client-side. +- Search network failure: fields untouched, and an inline hint says the lookup service could not be reached (distinct from the no-results message, which means the service answered). **Revised in code review 2026-07-24** — this originally said "silent-ish degrade", which in practice left the DOM byte-identical to the pre-click state, so a traveller on flaky mobile data could not tell a failed lookup from a broken button. A non-2xx response is also now treated as a failure rather than parsed as an empty result set. +- Invalid manual `lat`/`lng` text: the visual mismatch flag is the primary feedback, **and** an unresolved flag blocks submit. **Revised in code review 2026-07-24** — this originally said "no client-side hard block", on the stated grounds that server-side `cleanCoordinate()` was already the safety net. It was not: `cleanCoordinate()` had never been committed, so nothing validated coordinates anywhere. It now ships (`cache-on-save.php`, both the `/post` and Admin2 paths), so the two are genuine defence in depth rather than one imaginary net. Client-side parsing is deliberately *stricter* than the server's `is_numeric` (whole-value decimals only), which is the safe direction for a mismatch. - Geolocation permission denied: unchanged existing behavior (`#location-status` error message). ## Out of scope / explicitly deferred diff --git a/user b/user index 13c76b2..e873a9c 160000 --- a/user +++ b/user @@ -1 +1 @@ -Subproject commit 13c76b29a840e8082427e2e29c7cdaeb1ef895b6 +Subproject commit e873a9cb2341008c3d0fbe481e79000cac729c9d