fix(review): land cleanCoordinate server-side guard; surface geocode failures

Two code-review follow-ups.

cleanCoordinate() — the server-side coordinate sanitizer the design doc and
plan both describe as already shipped — had never actually been committed; it
existed only as uncommitted work in another checkout, so this branch had no
server-side validation of lat/lng at all (the blueprint fields are plain
`type: text` with no `validate:` key). Landing it here makes the spec's stated
safety net real. Also corrected its onAdminSave comment, which justified that
hook by saying the public form's lat/lng inputs are CSS-hidden and GPS-filled
— true before this feature, inverted by it. Both hooks are needed: this branch
makes /post the primary hand-entry path, not Admin2.

The geocode lookup swallowed every failure and reset the button, leaving the
DOM byte-identical to the pre-click state — a traveller on flaky mobile data
could not distinguish a failed lookup from a broken button. It now shows a
distinct hint, and checks r.ok first so a 4xx/5xx body no longer parses as
"no results" and tells the traveller their city does not exist. R8's actual
guarantee (fields untouched on failure) is preserved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

plugins/cache-on-save/cache-on-save.php

@@ -3,6 +3,7 @@ namespace Grav\Plugin;
 
 use Grav\Common\Cache;
 use Grav\Common\Data\ValidationException;
+use Grav\Common\Page\Interfaces\PageInterface;
 use Grav\Common\Plugin;
 use RocketTheme\Toolbox\Event\Event;
 
@@ -43,6 +44,16 @@ class CacheOnSavePlugin extends Plugin
             // (priority 0) has created the page and copied the uploaded files —
             // we reorder those files, then clear the page-tree cache.
             'onFormProcessed' => ['onFormProcessed', -100],
+            // Fired by the api plugin (PagesController) on every Admin2-driven
+            // create/update/translate, right before $page->save().
+            // onFormValidationProcessed (above) does not fire for Admin2 saves,
+            // so BOTH hooks are needed to cover every path lat/lng can arrive
+            // through: this one for Admin2/API, that one for the public /post
+            // form. (Until the location-override panel shipped, the public
+            // form's lat/lng inputs were CSS-hidden and GPS-filled, making
+            // Admin2 the only realistic hand-entry route. They are now visible
+            // and directly editable, so the /post path is the primary one.)
+            'onAdminSave' => ['onAdminSave', 0],
         ];
     }
 
@@ -74,6 +85,7 @@ class CacheOnSavePlugin extends Plugin
         }
 
         $form->setData('parent', $this->resolveDailiesParent($activeTrip));
+        $this->sanitizeCoordinates($form);
 
         // One shared /post form drives both create and edit (KTD1). add-page-by-form
         // reads overwrite_mode from the /post page header's pageconfig (not form
@@ -149,6 +161,79 @@ class CacheOnSavePlugin extends Plugin
         return '/' . $trip . '/dailies';
     }
 
+    /**
+     * Strip invisible Unicode formatting characters (bidi marks, zero-width
+     * joiners, etc. — Unicode category "Cf") from a pasted lat/lng value, then
+     * validate the result is a real coordinate.
+     *
+     * Root cause this guards against: coordinates copied from a maps app can
+     * carry an invisible mark (e.g. U+200E LEFT-TO-RIGHT MARK) that neither
+     * JS `.trim()` nor PHP's numeric-string coercion strip. Twig's
+     * `number_format` filter then silently float-casts the corrupted string
+     * to 0.0 (PHP does not raise a warning), placing the entry at Null Island
+     * instead of failing loudly.
+     *
+     * Coordinates are optional (some entries intentionally have none), so a
+     * blank value cleans to '' with no error — only a non-blank value that
+     * still fails to parse as an in-range decimal after cleaning is rejected.
+     *
+     * @return string the cleaned value to write back
+     * @throws ValidationException if non-blank but still invalid after cleaning
+     */
+    private function cleanCoordinate(string $field, string $raw, float $bound): string
+    {
+        $clean = trim(preg_replace('/\p{Cf}/u', '', $raw) ?? $raw);
+        if ($clean === '') {
+            return $clean;
+        }
+        if (!is_numeric($clean) || abs((float) $clean) > $bound) {
+            throw new ValidationException(sprintf(
+                '%s "%s" is not a valid coordinate — check for stray characters from pasting.',
+                $field === 'lat' ? 'Latitude' : 'Longitude',
+                $raw
+            ));
+        }
+        return $clean;
+    }
+
+    /** Post-form entry point (see cleanCoordinate) — 'lat'/'lng' as top-level form fields. */
+    private function sanitizeCoordinates($form): void
+    {
+        foreach (['lat' => 90.0, 'lng' => 180.0] as $field => $bound) {
+            $raw = $form->value($field);
+            if (!is_string($raw)) {
+                continue;
+            }
+            $form->setData($field, $this->cleanCoordinate($field, $raw, $bound));
+        }
+    }
+
+    /**
+     * Admin2/API entry point (see cleanCoordinate) — 'header.lat'/'header.lng'
+     * on the Page object the api plugin is about to save. Fires on every
+     * create/update/translate (PagesController::create/update/translatePage),
+     * so this also re-validates already-clean values on every subsequent edit
+     * — harmless, since a clean value round-trips unchanged.
+     */
+    public function onAdminSave(Event $event): void
+    {
+        $page = $event['page'] ?? $event['object'] ?? null;
+        if (!$page instanceof PageInterface) {
+            return;
+        }
+        $header = $page->header();
+        if (!$header) {
+            return;
+        }
+        foreach (['lat' => 90.0, 'lng' => 180.0] as $field => $bound) {
+            $raw = $header->{$field} ?? null;
+            if (!is_string($raw)) {
+                continue; // unset, null, or already a native number — nothing to clean
+            }
+            $header->{$field} = $this->cleanCoordinate($field, $raw, $bound);
+        }
+    }
+
     /**
      * The photo order the user arranged in the form, sent explicitly by
      * post-form.js as a JSON array of filenames in the dedicated

themes/intotheeast/js/src/location-map.js

@@ -26,7 +26,7 @@ var cached = null; // { container, promise }
 
 function buildPinElement() {
     var el = document.createElement('div');
-    el.className = 'location-pin'; // visual styling lives in style.css
+    el.className = 'location-pin'; // visual styling lives in post-form.css
     return el;
 }
 
@@ -58,7 +58,9 @@ export function getOrCreateLocationMap(container, onDragEnd) {
                 if (!pinSet) { marker.addTo(map); pinSet = true; }
                 map.panTo([lng, lat]);
             },
-            hasPin: function () { return pinSet; },
+            clearPin: function () {
+                if (pinSet) { marker.remove(); pinSet = false; }
+            },
             resize: function () { map.resize(); }
         };
     }).catch(function (err) {

themes/intotheeast/js/src/post-form.css

@@ -522,9 +522,12 @@
 
 /* "More location details" disclosure — search + map preview for setting an
    entry's coordinates without live GPS. Mirrors .more-options's disclosure
-   look (above); the lat/lng fields (relocated here by JS) and the lookup
-   button reuse style.css's existing .btn-action/.form-status/.field-invalid
-   conventions unmodified. */
+   look (above); the lookup button reuses style.css's existing .btn-action /
+   .form-status conventions unmodified. The relocated lat/lng fields do NOT
+   reuse .field-invalid — the panel adds its own .location-field--mismatch /
+   .location-field-note pair below, because the mismatch state is advisory and
+   carries live aria-invalid / aria-describedby wiring that showError() does
+   not. Worth consolidating with .field-invalid if that ever gains the same. */
 .location-details {
     margin-bottom: var(--space-5);
     border: 1px solid var(--color-border);

themes/intotheeast/js/src/post-form.js

@@ -607,12 +607,29 @@ function initLocationDetails() {
     // ── Map + sync (U4, U5, R9-R14, KTD1/KTD3/KTD4) ──
     var mapHandle = null;
 
+    // parseFloat is a PREFIX parser and must not be used here: '48abc' → 48,
+    // '48,85' → 48 (comma-decimal paste), '35.0116S' → +35.0116 (hemisphere
+    // suffix silently flipped to the wrong side of the equator). All three pass
+    // an isFinite + range check, clear the flag, and post the original string —
+    // the exact silent-corruption class this feature exists to end. Require the
+    // whole value to be a plain decimal instead. Not bare Number(): Number('')
+    // is 0, which would make a blank pair "valid" and pin it at Null Island.
+    function parseCoord(v) {
+        var s = String(v).trim();
+        if (!s || !/^[+-]?(\d+(\.\d*)?|\.\d+)$/.test(s)) return NaN;
+        return Number(s);
+    }
+
     function syncFields() {
-        var lat = parseFloat(latEl.value);
-        var lng = parseFloat(lngEl.value);
+        var lat = parseCoord(latEl.value);
+        var lng = parseCoord(lngEl.value);
         var valid = isFinite(lat) && isFinite(lng) &&
             lat >= -90 && lat <= 90 && lng >= -180 && lng <= 180;
         if (valid) {
+            // Normalise to the same precision the GPS handler and onDragEnd
+            // write, so what the pin shows is exactly what gets submitted.
+            latEl.value = lat.toFixed(6);
+            lngEl.value = lng.toFixed(6);
             clearMismatch();
             if (mapHandle) mapHandle.setPin(lat, lng);
         } else if (latEl.value.trim() || lngEl.value.trim()) {
@@ -621,8 +638,10 @@ function initLocationDetails() {
             setMismatch();
         } else {
             // Both blanked back out after being flagged — nothing left to submit,
-            // so the flag no longer applies.
+            // so the flag no longer applies. Drop the pin too: leaving it behind
+            // would present a stale coordinate as if it were still the entry's.
             clearMismatch();
+            if (mapHandle) mapHandle.clearPin();
         }
     }
     syncPinFromFields = syncFields; // expose for initGeo()'s GPS handler
@@ -643,6 +662,10 @@ function initLocationDetails() {
             syncFields();
         }).catch(function () {
             setHint('Map preview unavailable — you can still enter coordinates directly.');
+            // Still validate: syncFields tolerates a null mapHandle, and without
+            // this an offline traveller could open the panel and submit an
+            // invalid coordinate that was never flagged.
+            syncFields();
         });
     });
 
@@ -696,7 +719,14 @@ function initLocationDetails() {
         // button stuck disabled on "Searching…" forever.
         var controller = new AbortController();
         var timeoutId = setTimeout(function () { controller.abort(); }, 10000);
-        fetch(url, { signal: controller.signal }).then(function (r) { return r.json(); }).then(function (data) {
+        fetch(url, { signal: controller.signal }).then(function (r) {
+            // Without this a 4xx/5xx body (rate limit, upstream error) parses as
+            // JSON with no `results` key and the traveller is told their city
+            // does not exist — sending them off to hunt a spelling mistake that
+            // isn't there. Route real failures to the catch instead.
+            if (!r.ok) throw new Error('geocode http ' + r.status);
+            return r.json();
+        }).then(function (data) {
             var list = (data && data.results) || [];
             if (!list.length) {
                 setHint('No matches — try adding a country, or drag the pin on the map.');
@@ -723,14 +753,27 @@ function initLocationDetails() {
             }
             showResults(list);
         }).catch(function () {
-            // R8: network failure (including our own timeout abort) degrades
-            // silently — fields untouched.
+            // R8 revised: the fields stay untouched on failure (that part of R8
+            // is the actual guarantee), but the failure is no longer invisible.
+            // Silence left the DOM byte-identical to the pre-click state — empty
+            // hint, enabled button — so a traveller on flaky mobile data could
+            // not tell "lookup failed" from "the button is broken". Distinct
+            // from the no-match message above, which means the service answered.
+            setHint('Couldn’t reach the lookup service — check your connection and try again, or drag the pin on the map.');
         }).then(function () {
             clearTimeout(timeoutId);
             lookupBtn.disabled = false;
             lookupBtn.textContent = LOOKUP_LABEL;
         });
     });
+
+    // Validate whatever is already in the fields at boot. initDraft() restores
+    // data[lat]/data[lng] from localStorage before this runs, and edit-mode
+    // prefills them asynchronously — both by direct .value assignment, which
+    // fires no events. Without this the submit gate in initValidation (which
+    // keys on .location-field--mismatch) sees no flag and lets a restored
+    // invalid coordinate straight through.
+    syncFields();
 }
 
 /* ── Blocking required-field validation (U5, R19) ─────────────
@@ -1418,6 +1461,11 @@ function initEditMode() {
             editSetContent(d.content);
             editSetText('lat', h.lat);
             editSetText('lng', h.lng);
+            // Direct .value writes fire no events, so flag/pin state would stay
+            // stale — and an entry stored with an out-of-range coordinate would
+            // never be flagged. initLocationDetails() has already replaced the
+            // no-op by the time this async prefill resolves.
+            syncPinFromFields();
             editSetText('location_city', h.location_city);
             editSetText('location_country', h.location_country);
             editSetText('weather_desc', h.weather_desc);