fix(post): close location-map race, block submit on bad coords
Code review (4 independent reviewers) converged on the same bug: the maplibre-gl singleton cached its handle only after import() resolved, so a fast close/reopen of the "More location details" panel could race two Map instances onto one container. Cache the in-flight promise synchronously instead, and propagate/handle import rejection so a failed map load surfaces a hint instead of hanging silently. Also closes a submit-time hole the adversarial pass found: the mismatch flag on lat/lng was purely cosmetic and never blocked form submission, so out-of-range coordinates could be saved. The flag now gates submit like the other required fields, and clears itself when both fields are blanked back out instead of sticking. The geocode fetch gets a 10s timeout via AbortController so a hung response can't leave the lookup button disabled forever. Also moves the location-details CSS out of the site-wide style.css into post-form's own code-split stylesheet, since none of it is used outside the post form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Claude Sonnet 5
File diff suppressed because one or more lines are too long
@@ -12,10 +12,17 @@
|
||||
* which calls this on every open and then always calls the returned
|
||||
* handle's resize() (the container sits under display:none while the panel
|
||||
* is closed, so the first paint would otherwise get a zero-size canvas).
|
||||
*
|
||||
* The cache stores the in-flight PROMISE, not just the resolved handle —
|
||||
* written synchronously before import() settles. A close/reopen of the panel
|
||||
* while the maplibre-gl chunk is still loading would otherwise re-enter this
|
||||
* function and race a second import().then() into building a second Map
|
||||
* against the same container (caught in code review — confirmed independently
|
||||
* by four reviewers).
|
||||
*/
|
||||
import { MAP_STYLE } from './map-style.js';
|
||||
|
||||
var cached = null; // { container, handle }
|
||||
var cached = null; // { container, promise }
|
||||
|
||||
function buildPinElement() {
|
||||
var el = document.createElement('div');
|
||||
@@ -25,10 +32,10 @@ function buildPinElement() {
|
||||
|
||||
export function getOrCreateLocationMap(container, onDragEnd) {
|
||||
if (cached && cached.container === container) {
|
||||
return Promise.resolve(cached.handle);
|
||||
return cached.promise;
|
||||
}
|
||||
|
||||
return import('maplibre-gl').then(function (mod) {
|
||||
var promise = import('maplibre-gl').then(function (mod) {
|
||||
var maplibregl = mod.default || mod;
|
||||
var map = new maplibregl.Map({
|
||||
container: container,
|
||||
@@ -45,7 +52,7 @@ export function getOrCreateLocationMap(container, onDragEnd) {
|
||||
if (onDragEnd) onDragEnd(marker.getLngLat());
|
||||
});
|
||||
|
||||
var handle = {
|
||||
return {
|
||||
setPin: function (lat, lng) {
|
||||
marker.setLngLat([lng, lat]);
|
||||
if (!pinSet) { marker.addTo(map); pinSet = true; }
|
||||
@@ -54,8 +61,14 @@ export function getOrCreateLocationMap(container, onDragEnd) {
|
||||
hasPin: function () { return pinSet; },
|
||||
resize: function () { map.resize(); }
|
||||
};
|
||||
|
||||
cached = { container: container, handle: handle };
|
||||
return handle;
|
||||
}).catch(function (err) {
|
||||
// Leave the cache clear so a later retry (e.g. after transient network
|
||||
// failure) re-attempts the import instead of permanently returning a
|
||||
// rejected promise for this container.
|
||||
if (cached && cached.container === container) cached = null;
|
||||
throw err;
|
||||
});
|
||||
|
||||
cached = { container: container, promise: promise };
|
||||
return promise;
|
||||
}
|
||||
|
||||
@@ -519,3 +519,107 @@
|
||||
/* SortableJS drag feedback. */
|
||||
.photo-editor__cell.sortable-ghost { opacity: 0.4; }
|
||||
.photo-editor__cell.sortable-chosen { outline: 2px solid var(--color-accent); }
|
||||
|
||||
/* "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. */
|
||||
.location-details {
|
||||
margin-bottom: var(--space-5);
|
||||
border: 1px solid var(--color-border);
|
||||
border-radius: var(--radius-md);
|
||||
background: var(--color-canvas);
|
||||
}
|
||||
.location-details__summary {
|
||||
cursor: pointer;
|
||||
padding: 0.875rem 1rem;
|
||||
min-height: 44px;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
font-family: var(--font-ui);
|
||||
font-size: var(--text-sm);
|
||||
font-weight: 600;
|
||||
color: var(--color-ink);
|
||||
list-style: none;
|
||||
user-select: none;
|
||||
}
|
||||
.location-details__summary::-webkit-details-marker { display: none; }
|
||||
.location-details__summary::before {
|
||||
content: '▸';
|
||||
margin-right: var(--space-2);
|
||||
color: var(--color-ink-muted);
|
||||
transition: transform 0.15s;
|
||||
}
|
||||
.location-details[open] .location-details__summary::before { transform: rotate(90deg); }
|
||||
.location-details[open] .location-details__summary { border-bottom: 1px solid var(--color-border); }
|
||||
.location-details > .form-field { padding: 0 1rem; }
|
||||
.location-details > .form-field:first-of-type { padding-top: var(--space-4); }
|
||||
.location-details > .form-field:last-of-type { padding-bottom: var(--space-2); }
|
||||
.location-details__body { padding: 1rem; }
|
||||
|
||||
.location-search-row { display: flex; gap: var(--space-3); align-items: center; flex-wrap: wrap; }
|
||||
|
||||
.location-search-hint {
|
||||
font-size: var(--text-sm);
|
||||
color: var(--color-ink-muted);
|
||||
margin-top: var(--space-2);
|
||||
}
|
||||
.location-search-hint:empty { display: none; }
|
||||
|
||||
.location-search-results {
|
||||
list-style: none;
|
||||
margin: var(--space-3) 0 0;
|
||||
padding: 0;
|
||||
border: 1px solid var(--color-border);
|
||||
border-radius: var(--radius-md);
|
||||
overflow: hidden;
|
||||
}
|
||||
.location-search-results:empty { display: none; margin: 0; border: none; }
|
||||
.location-search-results li + li { border-top: 1px solid var(--color-border); }
|
||||
.location-search-results button {
|
||||
display: block;
|
||||
width: 100%;
|
||||
text-align: left;
|
||||
padding: 0.75rem 1rem;
|
||||
min-height: 44px;
|
||||
background: var(--color-canvas);
|
||||
border: none;
|
||||
font-family: var(--font-ui);
|
||||
font-size: var(--text-sm);
|
||||
color: var(--color-ink);
|
||||
cursor: pointer;
|
||||
}
|
||||
.location-search-results button:hover,
|
||||
.location-search-results button:focus-visible { background: var(--color-paper); }
|
||||
|
||||
.location-map {
|
||||
position: relative;
|
||||
width: 100%;
|
||||
height: 240px;
|
||||
margin-top: var(--space-4);
|
||||
border-radius: var(--radius-md);
|
||||
overflow: hidden;
|
||||
background: var(--color-paper);
|
||||
}
|
||||
.location-map .maplibregl-canvas { border-radius: var(--radius-md); }
|
||||
|
||||
.location-pin {
|
||||
width: 44px;
|
||||
height: 44px;
|
||||
border-radius: 50%;
|
||||
background: var(--color-accent);
|
||||
border: 3px solid #fff;
|
||||
box-shadow: 0 1px 6px rgba(0, 0, 0, 0.5);
|
||||
cursor: grab;
|
||||
}
|
||||
.location-pin:active { cursor: grabbing; }
|
||||
|
||||
/* Mismatch flag: a typed lat/lng that doesn't (yet) parse to a valid pin. */
|
||||
.location-field--mismatch { border-color: var(--color-error) !important; outline-color: var(--color-error) !important; }
|
||||
.location-field-note {
|
||||
display: block;
|
||||
font-size: var(--text-sm);
|
||||
color: var(--color-error);
|
||||
margin-top: var(--space-1);
|
||||
}
|
||||
|
||||
@@ -619,6 +619,10 @@ function initLocationDetails() {
|
||||
// Only flag once the traveller has actually typed something —
|
||||
// fresh blank fields are "no pin yet" (R12), not a mismatch.
|
||||
setMismatch();
|
||||
} else {
|
||||
// Both blanked back out after being flagged — nothing left to submit,
|
||||
// so the flag no longer applies.
|
||||
clearMismatch();
|
||||
}
|
||||
}
|
||||
syncPinFromFields = syncFields; // expose for initGeo()'s GPS handler
|
||||
@@ -637,6 +641,8 @@ function initLocationDetails() {
|
||||
mapHandle = handle;
|
||||
handle.resize(); // fixes the zero-size canvas from painting while display:none
|
||||
syncFields();
|
||||
}).catch(function () {
|
||||
setHint('Map preview unavailable — you can still enter coordinates directly.');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -686,7 +692,11 @@ function initLocationDetails() {
|
||||
lookupBtn.textContent = 'Searching…';
|
||||
var url = 'https://geocoding-api.open-meteo.com/v1/search?name=' +
|
||||
encodeURIComponent(query) + '&count=10&language=en&format=json';
|
||||
fetch(url).then(function (r) { return r.json(); }).then(function (data) {
|
||||
// Bound the request so a hung/never-resolving response can't leave the
|
||||
// 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) {
|
||||
var list = (data && data.results) || [];
|
||||
if (!list.length) {
|
||||
setHint('No matches — try adding a country, or drag the pin on the map.');
|
||||
@@ -713,8 +723,10 @@ function initLocationDetails() {
|
||||
}
|
||||
showResults(list);
|
||||
}).catch(function () {
|
||||
// R8: network failure degrades silently — fields untouched.
|
||||
// R8: network failure (including our own timeout abort) degrades
|
||||
// silently — fields untouched.
|
||||
}).then(function () {
|
||||
clearTimeout(timeoutId);
|
||||
lookupBtn.disabled = false;
|
||||
lookupBtn.textContent = LOOKUP_LABEL;
|
||||
});
|
||||
@@ -805,6 +817,13 @@ function initValidation() {
|
||||
if (!firstInvalid) firstInvalid = el;
|
||||
}
|
||||
});
|
||||
|
||||
// A flagged-but-unresolved lat/lng (typed garbage, never fixed or
|
||||
// cleared) must not reach the server — the mismatch styling alone
|
||||
// doesn't block submission (caught in code review).
|
||||
var mismatchEl = form.querySelector('.location-field--mismatch');
|
||||
if (mismatchEl && !firstInvalid) firstInvalid = mismatchEl;
|
||||
|
||||
if (firstInvalid) {
|
||||
e.preventDefault();
|
||||
if (typeof firstInvalid.focus === 'function') firstInvalid.focus();
|
||||
|
||||
Reference in New Issue
Block a user