mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-25 17:03:36 +00:00
fix(rx-coverage): open at the configured map default, with its own saved viewport (#2033)
The coverage page opened at a hardcoded [51.0, 4.8] zoom 8 regardless of deployment, ignoring /api/config/map (#2032). It now follows the same precedence as the main map (URL hash, then saved position, then /api/config/map, then [37.6, -122.1] zoom 9) and persists its own position across visits, syncing lat/lon/zoom into the hash so a view is shareable. The saved position lives under its own key, rx-coverage-view, and the page never reads or writes the main map's map-view: sharing the configured default was the bug, sharing the session position was not. Both suites assert map-view stays untouched after a pan, so reintroducing a shared write fails instead of passing quietly. Also fixed here: selectedRx is now percent-encoded into the hash, and a generation counter stops a late /api/config/map response or a stale 150ms layout timer from building a map for a page that was already left. Reviewed twice. Verified by mutation rather than by reading: writing map-view too, ignoring the saved coverage position, and dropping the /api/config/map fetch each make the unit suite exit 1, so it covers the feature, the fix and the rejected alternative. The deploy.yml invocation was confirmed to land inside Run Playwright E2E tests (fail-fast) by parsing the workflow, and CI run 35261689694 is the E2E suite's first real execution: Go prints 'RX coverage viewport regressions OK' and Playwright prints 'RX coverage viewport browser regressions OK'. Worth recording for the next reviewer: that E2E asserted localStorage.getItem('map-view'), so wiring it into deploy.yml without updating the assertion would have turned the job red on its first ever run. It was registered in scripts/non-unit-tests.json but invoked by nothing, the gap tracked as #2037. Merged by the interim maintainer without a second human reviewer: CI and the mutation checks above are the independent checks. Fixes #2032
This commit is contained in:
liquidraver
@@ -577,6 +577,7 @@ jobs:
|
||||
CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node tests/e2e/test-node-reach-coverage-e2e.js 2>&1 | tee -a e2e-output.txt
|
||||
CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node tests/e2e/test-issue-1640-compare-discovery-e2e.js 2>&1 | tee -a e2e-output.txt
|
||||
CHROMIUM_REQUIRE=1 node tests/e2e/test-neighbor-map-btn-clip-e2e.js 2>&1 | tee -a e2e-output.txt
|
||||
BASE_URL=http://localhost:13581 node tests/e2e/test-rx-coverage-viewport-e2e.js 2>&1 | tee -a e2e-output.txt
|
||||
|
||||
# #1616: slide-over focus-restore flake-gate. Runs the slide-over
|
||||
# E2E 20 consecutive times against the SAME backend instance so
|
||||
|
||||
+41
-7
@@ -6,7 +6,7 @@
|
||||
Fork-only feature; isolated page (no changes to the core map). */
|
||||
'use strict';
|
||||
(function () {
|
||||
var map = null, covLayer = null, days = 7, selectedRx = '', selectedName = '', boardCache = [], destroyed = false;
|
||||
var map = null, covLayer = null, days = 7, selectedRx = '', selectedName = '', boardCache = [], destroyed = false, generation = 0;
|
||||
|
||||
function cssColor(varName) {
|
||||
try { return getComputedStyle(document.documentElement).getPropertyValue(varName).trim() || '#888'; }
|
||||
@@ -215,21 +215,26 @@
|
||||
}
|
||||
|
||||
function syncHash() {
|
||||
var q = 'days=' + days + (selectedRx ? '&rx=' + selectedRx : '');
|
||||
var q = 'days=' + days + (selectedRx ? '&rx=' + encodeURIComponent(selectedRx) : '');
|
||||
if (map) {
|
||||
var c = map.getCenter();
|
||||
q += '&lat=' + c.lat.toFixed(5) + '&lon=' + c.lng.toFixed(5) + '&zoom=' + map.getZoom();
|
||||
}
|
||||
try { history.replaceState(null, '', '#/rx-coverage?' + q); } catch (e) {}
|
||||
}
|
||||
|
||||
function init(container) {
|
||||
destroyed = false;
|
||||
var current = ++generation;
|
||||
// A direct land on #/rx-coverage can run before MeshConfigReady resolves, at
|
||||
// which point MC_CLIENT_RX_COVERAGE is still undefined and the page would
|
||||
// wrongly show "not enabled". Defer until server config is loaded (#13).
|
||||
Promise.resolve(window.MeshConfigReady).then(function () {
|
||||
if (!destroyed) start(container);
|
||||
if (!destroyed && current === generation) start(container, current);
|
||||
});
|
||||
}
|
||||
|
||||
function start(container) {
|
||||
async function start(container, current) {
|
||||
if (!window.MC_CLIENT_RX_COVERAGE) {
|
||||
container.innerHTML = '<div class="nq-msg">Coverage is not enabled on this deployment.</div>';
|
||||
return;
|
||||
@@ -240,21 +245,50 @@
|
||||
if (p) { var dd = parseInt(p.get('days'), 10); if ([1, 7, 14, 30].indexOf(dd) >= 0) days = dd; selectedRx = (p.get('rx') || '').toLowerCase(); }
|
||||
} catch (e) {}
|
||||
container.innerHTML = pageHtml();
|
||||
map = L.map('rxMap', { zoomControl: true, attributionControl: false }).setView([51.0, 4.8], 8);
|
||||
// Initialize viewport: explicit URL hash, coverage page's saved position, deployment defaults (#2032).
|
||||
var viewport = parseViewportHash(location.hash);
|
||||
var explicitViewport = !!viewport;
|
||||
if (!viewport) {
|
||||
try {
|
||||
var saved = JSON.parse(localStorage.getItem('rx-coverage-view'));
|
||||
if (saved && saved.lat != null && saved.lng != null && saved.zoom != null) {
|
||||
viewport = parseViewportHash(new URLSearchParams({ lat: saved.lat, lon: saved.lng, zoom: saved.zoom }).toString());
|
||||
}
|
||||
} catch (e) {} // Storage may be disabled or contain an incomplete value.
|
||||
}
|
||||
if (!viewport) {
|
||||
viewport = { lat: 37.6, lon: -122.1, zoom: 9 };
|
||||
try {
|
||||
var response = await fetch('/api/config/map');
|
||||
var cfg = await response.json();
|
||||
if (cfg && Array.isArray(cfg.center) && cfg.center.length === 2) {
|
||||
viewport = parseViewportHash(new URLSearchParams({ lat: cfg.center[0], lon: cfg.center[1], zoom: cfg.zoom == null ? 9 : cfg.zoom }).toString()) || viewport;
|
||||
}
|
||||
} catch (e) {} // Match the main map's offline fallback.
|
||||
}
|
||||
if (destroyed || current !== generation) return;
|
||||
map = L.map('rxMap', { zoomControl: true, attributionControl: false }).setView([viewport.lat, viewport.lon], viewport.zoom);
|
||||
if (typeof window._applyTilesToNodeMap === 'function') window._applyTilesToNodeMap(map);
|
||||
else L.tileLayer('https://tile.openstreetmap.org/{z}/{x}/{y}.png', { maxZoom: 19 }).addTo(map);
|
||||
covLayer = L.layerGroup().addTo(map);
|
||||
// Debounce pan/zoom redraws so dragging the map doesn't fire a storm of
|
||||
// /api/rx-coverage requests (#6). Direct calls (setDays, fit) stay immediate.
|
||||
map.on('moveend zoomend', debounce(drawCoverage, 200));
|
||||
map.on('moveend zoomend', debounce(function () {
|
||||
if (destroyed || current !== generation || !map) return;
|
||||
var center = map.getCenter();
|
||||
try { localStorage.setItem('rx-coverage-view', JSON.stringify({ lat: center.lat, lng: center.lng, zoom: map.getZoom() })); } catch (e) {}
|
||||
syncHash();
|
||||
drawCoverage();
|
||||
}, 200));
|
||||
var bar = document.getElementById('rxDays');
|
||||
if (bar) bar.addEventListener('click', function (e) { var b = e.target.closest('button[data-days]'); if (b) setDays(+b.dataset.days); });
|
||||
setTimeout(function () { if (!destroyed && map) { map.invalidateSize(); if (selectedRx) fitToObserver(); else drawCoverage(); } }, 150);
|
||||
setTimeout(function () { if (!destroyed && current === generation && map) { map.invalidateSize(); if (selectedRx && !explicitViewport) fitToObserver(); else drawCoverage(); } }, 150);
|
||||
loadBoard();
|
||||
}
|
||||
|
||||
function destroy() {
|
||||
destroyed = true;
|
||||
generation++;
|
||||
if (map) { try { map.remove(); } catch (e) {} map = null; }
|
||||
covLayer = null;
|
||||
}
|
||||
|
||||
@@ -104,6 +104,7 @@
|
||||
"tests/e2e/test-packets-scope-column.js",
|
||||
"tests/e2e/test-path-inspector-coverage-e2e.js",
|
||||
"tests/e2e/test-rx-coverage-mobile-nav-e2e.js",
|
||||
"tests/e2e/test-rx-coverage-viewport-e2e.js",
|
||||
"tests/e2e/test-show-neighbors.js",
|
||||
"tests/e2e/test-slideover-1056-e2e.js",
|
||||
"tests/e2e/test-slideover-1168-munger-e2e.js",
|
||||
|
||||
@@ -174,6 +174,7 @@ node tests/unit/test-pull-to-reconnect.js
|
||||
node tests/unit/test-repeater-metric-scatter.js
|
||||
node tests/unit/test-rx-coverage-config-race.js
|
||||
node tests/unit/test-rx-coverage-escape.js
|
||||
node tests/unit/test-rx-coverage-viewport.js
|
||||
node tests/unit/test-scope-audit-styles-linked.js
|
||||
node tests/unit/test-slideover-1056-rowsel-strict.js
|
||||
node tests/unit/test-top-routes-overlay.js
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
#!/usr/bin/env node
|
||||
'use strict';
|
||||
// #2032: real Leaflet viewport initialization, persistence and observer links.
|
||||
const assert = require('assert');
|
||||
const { chromium } = require('playwright');
|
||||
const BASE = process.env.BASE_URL || 'http://localhost:3000';
|
||||
(async () => {
|
||||
const browser = await chromium.launch({ headless: true, executablePath: process.env.CHROMIUM_PATH || undefined });
|
||||
try {
|
||||
const page = await browser.newPage();
|
||||
const errors = [];
|
||||
page.on('pageerror', error => errors.push(error.message));
|
||||
await page.addInitScript(() => {
|
||||
let leaflet;
|
||||
Object.defineProperty(window, 'L', {
|
||||
configurable: true,
|
||||
get: () => leaflet,
|
||||
set(value) {
|
||||
leaflet = value;
|
||||
value.Map.addInitHook(function () { window.__rxTestMap = this; });
|
||||
}
|
||||
});
|
||||
});
|
||||
await page.route('**/api/config/client', async route => {
|
||||
const response = await route.fetch();
|
||||
await route.fulfill({ json: { ...await response.json(), clientRxCoverage: true } });
|
||||
});
|
||||
await page.route('**/api/config/map', route => route.fulfill({ json: { center: [12, 34], zoom: 6 } }));
|
||||
await page.route('**/api/rx-leaderboard?*', route => route.fulfill({ json: { observers: [] } }));
|
||||
let worldRequests = 0;
|
||||
const observerCell = { type: 'Feature', geometry: { type: 'Polygon', coordinates: [[[40, 10], [41, 10], [41, 11], [40, 11], [40, 10]]] }, properties: { count: 1, has_sig: false, nodes: [] } };
|
||||
await page.route('**/api/rx-coverage?*', route => {
|
||||
if (new URL(route.request().url()).searchParams.get('bbox') === '-90,-180,90,180') worldRequests++;
|
||||
return route.fulfill({ json: { type: 'FeatureCollection', features: [observerCell] } });
|
||||
});
|
||||
async function waitMap() {
|
||||
await page.waitForFunction(() => window.__rxTestMap && window.__rxTestMap._loaded && document.querySelector('#rxMap.leaflet-container'));
|
||||
}
|
||||
async function viewport() {
|
||||
return page.evaluate(() => { const m = window.__rxTestMap, c = m.getCenter(); return { lat: c.lat, lng: c.lng, zoom: m.getZoom() }; });
|
||||
}
|
||||
function assertViewport(actual, expected) {
|
||||
// Leaflet projects centers to pixels during invalidateSize/pan, so allow
|
||||
// sub-pixel coordinate rounding while still checking the intended view.
|
||||
assert(Math.abs(actual.lat - expected.lat) < 0.001, JSON.stringify(actual));
|
||||
assert(Math.abs(actual.lng - expected.lng) < 0.001, JSON.stringify(actual));
|
||||
assert.equal(actual.zoom, expected.zoom);
|
||||
}
|
||||
await page.goto(BASE + '/#/rx-coverage'); await waitMap();
|
||||
assertViewport(await viewport(), { lat: 12, lng: 34, zoom: 6 });
|
||||
await page.evaluate(() => window.__rxTestMap.setView([22, 44], 10, { animate: false }));
|
||||
await page.waitForFunction(() => Math.abs(Number(new URLSearchParams(location.hash.split('?')[1]).get('lat')) - 22) < 0.001);
|
||||
assertViewport(await page.evaluate(() => JSON.parse(localStorage.getItem('rx-coverage-view'))), { lat: 22, lng: 44, zoom: 10 });
|
||||
assert.equal(await page.evaluate(() => localStorage.getItem('map-view')), null, 'coverage must not write the main map\'s saved viewport');
|
||||
await page.reload(); await waitMap(); assertViewport(await viewport(), { lat: 22, lng: 44, zoom: 10 });
|
||||
// Remove the shareable URL to independently verify this page's saved state.
|
||||
await page.goto(BASE + '/#/rx-coverage'); await page.reload(); await waitMap();
|
||||
assertViewport(await viewport(), { lat: 22, lng: 44, zoom: 10 });
|
||||
await page.goto(BASE + '/#/rx-coverage?days=14&rx=abcd&lat=0&lon=0&zoom=5'); await page.reload(); await waitMap();
|
||||
await page.waitForTimeout(500);
|
||||
assertViewport(await viewport(), { lat: 0, lng: 0, zoom: 5 });
|
||||
assert.equal(worldRequests, 0, 'explicit observer viewport must not auto-fit');
|
||||
await page.click('#rxDays button[data-days="30"]');
|
||||
const params = new URLSearchParams(new URL(page.url()).hash.split('?')[1]);
|
||||
assert.equal(params.get('days'), '30'); assert.equal(params.get('rx'), 'abcd'); assert.equal(params.get('lat'), '0.00000');
|
||||
// Observer-only links still fit the observer's complete extent.
|
||||
await page.goto(BASE + '/#/rx-coverage?rx=abcd'); await page.reload(); await waitMap();
|
||||
await page.waitForFunction(() => {
|
||||
const c = window.__rxTestMap.getCenter();
|
||||
return c.lat > 10 && c.lat < 11 && c.lng > 40 && c.lng < 41;
|
||||
});
|
||||
assert(worldRequests > 0, 'observer-only links must request full extent');
|
||||
if (process.env.SCREENSHOT_PATH) await page.screenshot({ path: process.env.SCREENSHOT_PATH, fullPage: true });
|
||||
assert.deepStrictEqual(errors, []);
|
||||
console.log('RX coverage viewport browser regressions OK');
|
||||
} finally { await browser.close(); }
|
||||
})().catch(error => { console.error(error); process.exit(1); });
|
||||
@@ -0,0 +1,95 @@
|
||||
'use strict';
|
||||
const REPO_ROOT = require('path').resolve(__dirname, '..', '..');
|
||||
// #2032: exercise the registered RX page, with the real shared URL parser.
|
||||
const assert = require('assert');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const vm = require('vm');
|
||||
const source = fs.readFileSync(path.join(REPO_ROOT, 'public', 'rx-coverage.js'), 'utf8');
|
||||
const app = fs.readFileSync(path.join(REPO_ROOT, 'public', 'app.js'), 'utf8');
|
||||
const parser = app.slice(app.indexOf('function parseViewportHash('), app.indexOf("if (typeof window !== 'undefined') { window.parseViewportHash"));
|
||||
const flush = () => new Promise(resolve => setImmediate(resolve));
|
||||
function harness(options = {}) {
|
||||
let page;
|
||||
const maps = [], requests = [], timers = [];
|
||||
const storage = { 'rx-coverage-view': options.saved };
|
||||
const location = { hash: options.hash || '#/rx-coverage' };
|
||||
const context = {
|
||||
URLSearchParams, location, Promise, console,
|
||||
window: { MC_CLIENT_RX_COVERAGE: true, MeshConfigReady: options.ready },
|
||||
document: { getElementById: () => null },
|
||||
registerPage: (_, value) => { page = value; },
|
||||
getHashParams: () => new URLSearchParams(location.hash.split('?')[1]),
|
||||
localStorage: {
|
||||
getItem: key => { if (options.blockStorage) throw Error('blocked'); return storage[key]; },
|
||||
setItem: (key, value) => { if (options.blockStorage) throw Error('blocked'); storage[key] = value; }
|
||||
},
|
||||
history: { replaceState: (_, __, hash) => { location.hash = hash; } },
|
||||
debounce: fn => fn,
|
||||
setTimeout: fn => { timers.push(fn); }, clearTimeout: () => {},
|
||||
fetch: url => {
|
||||
requests.push(url);
|
||||
if (url === '/api/config/map') {
|
||||
if (options.configPromise) return options.configPromise;
|
||||
if (options.failConfig) return Promise.reject(Error('offline'));
|
||||
return Promise.resolve({ json: () => options.config || { center: [12, 34], zoom: 6 } });
|
||||
}
|
||||
return Promise.resolve({ json: () => ({ features: [], observers: [] }) });
|
||||
},
|
||||
L: {
|
||||
map: () => {
|
||||
const m = { events: {}, setView(center, zoom) { this.center = Array.from(center); this.zoom = zoom; return this; },
|
||||
on(events, fn) { events.split(' ').forEach(event => { this.events[event] = fn; }); },
|
||||
getCenter() { return { lat: this.center[0], lng: this.center[1] }; }, getZoom() { return this.zoom; },
|
||||
getBounds() { return { getSouth: () => 0, getWest: () => 0, getNorth: () => 1, getEast: () => 1 }; },
|
||||
invalidateSize() {}, remove() { this.removed = true; } };
|
||||
maps.push(m); return m;
|
||||
},
|
||||
tileLayer: () => ({ addTo() {} }), layerGroup: () => ({ addTo() { return this; }, clearLayers() {} })
|
||||
}
|
||||
};
|
||||
vm.runInNewContext(parser + '\n' + source, context);
|
||||
return { page, maps, requests, timers, storage, location, init: async () => { page.init({ innerHTML: '' }); await flush(); } };
|
||||
}
|
||||
(async () => {
|
||||
let h = harness(); await h.init(); assert.deepStrictEqual(h.maps[0].center, [12, 34]); assert.equal(h.maps[0].zoom, 6); assert.equal(h.requests.filter(url => url === '/api/config/map').length, 1);
|
||||
h = harness({ saved: JSON.stringify({ lat: 22, lng: 44, zoom: 11 }) }); await h.init(); assert.deepStrictEqual(h.maps[0].center, [22, 44]); assert.equal(h.maps[0].zoom, 11); assert(!h.requests.includes('/api/config/map'));
|
||||
h = harness({ hash: '#/rx-coverage?days=14&rx=abcd&lat=0&lon=0&zoom=5', saved: JSON.stringify({ lat: 22, lng: 44, zoom: 11 }) });
|
||||
await h.init(); assert.deepStrictEqual(h.maps[0].center, [0, 0]); assert(!h.requests.includes('/api/config/map')); h.timers.forEach(fn => fn()); await flush();
|
||||
assert(!h.requests.some(url => url.includes('bbox=-90,-180')), 'explicit URL viewport must not be fitted to observer');
|
||||
h.maps[0].setView([40, 50], 10); h.maps[0].events.moveend();
|
||||
assert.deepStrictEqual(JSON.parse(h.storage['rx-coverage-view']), { lat: 40, lng: 50, zoom: 10 });
|
||||
assert.equal(h.storage['map-view'], undefined, 'coverage must not write the main map\'s saved viewport');
|
||||
const params = new URLSearchParams(h.location.hash.split('?')[1]);
|
||||
for (const [key, value] of Object.entries({ days: '14', rx: 'abcd', lat: '40.00000', lon: '50.00000', zoom: '10' })) assert.equal(params.get(key), value);
|
||||
for (const saved of ['broken', 'null', '{}', '{"lat":12,"lng":34,"zoom":"invalid"}', '{"lat":null,"lng":4,"zoom":8}', '{"lat":91,"lng":4,"zoom":8}', '{"lat":"nope","lng":4,"zoom":8}']) {
|
||||
h = harness({ saved }); await h.init(); assert.deepStrictEqual(h.maps[0].center, [12, 34], saved);
|
||||
}
|
||||
h = harness({ saved: JSON.stringify({ lat: 0, lng: 0, zoom: 4 }) }); await h.init(); assert.deepStrictEqual(h.maps[0].center, [0, 0]);
|
||||
h = harness({ hash: '#/rx-coverage?lat=bad&lon=34' }); await h.init(); assert.deepStrictEqual(h.maps[0].center, [12, 34]);
|
||||
for (const center of [[999, 34], null, [null, 34], ['bad', 34], []]) {
|
||||
h = harness({ config: { center, zoom: 6 } }); await h.init(); assert.deepStrictEqual(h.maps[0].center, [37.6, -122.1]);
|
||||
}
|
||||
h = harness({ blockStorage: true }); await h.init(); h.maps[0].events.moveend();
|
||||
h = harness({ failConfig: true }); await h.init(); assert.deepStrictEqual(h.maps[0].center, [37.6, -122.1]); assert.equal(h.maps[0].zoom, 9);
|
||||
h = harness({ hash: '#/rx-coverage?rx=abcd' }); await h.init(); h.timers.forEach(fn => fn()); await flush(); assert(h.requests.some(url => url.includes('bbox=-90,-180')));
|
||||
let resolve;
|
||||
h = harness({ configPromise: new Promise(r => { resolve = r; }) });
|
||||
await h.init(); h.page.destroy(); resolve({ json: () => ({ center: [12, 34], zoom: 6 }) }); await flush(); assert.equal(h.maps.length, 0, 'late config must not recreate a destroyed map');
|
||||
let delayed;
|
||||
h = harness({ configPromise: new Promise(r => { delayed = r; }) });
|
||||
await h.init(); h.page.destroy(); h.page.init({ innerHTML: '' }); await flush();
|
||||
delayed({ json: () => ({ center: [12, 34], zoom: 6 }) }); await flush();
|
||||
assert.equal(h.maps.length, 1, 'old config response must not create a map after re-entry');
|
||||
// A stale 150 ms layout timer must not fetch for the next visit.
|
||||
const oldMove = h.maps[0].events.moveend;
|
||||
const oldTimer = h.timers[0]; h.page.destroy(); h.page.init({ innerHTML: '' }); await flush();
|
||||
const before = h.requests.length; oldTimer(); assert.equal(h.requests.length, before);
|
||||
h.maps[1].setView([30, 60], 8);
|
||||
const oldHash = h.location.hash, oldSaved = h.storage['rx-coverage-view'];
|
||||
oldMove(); assert.equal(h.location.hash, oldHash); assert.equal(h.storage['rx-coverage-view'], oldSaved); assert.equal(h.requests.length, before);
|
||||
let ready;
|
||||
h = harness({ ready: new Promise(r => { ready = r; }) });
|
||||
h.page.init({ innerHTML: '' }); h.page.destroy(); h.page.init({ innerHTML: '' }); ready(); await flush(); assert.equal(h.maps.length, 1, 'old initialization must not survive destroy/re-entry');
|
||||
console.log('RX coverage viewport regressions OK');
|
||||
})().catch(error => { console.error(error); process.exit(1); });
|
||||
Reference in New Issue
Block a user