fix(map) :: ignore map coordinates that are not a pair of numbers (#1368)

Author: 81reap

CHANGELOG.md

@@ -6,6 +6,7 @@
  - Added a `toast` component with plain-text or Markdown content, icons, colors, six screen placements, configurable auto-dismiss timing, optional manual dismissal, URL-fragment triggers, and automatic stacking of queued notifications.
  - `sqlpage.send_mail` now supports rich email bodies. Use `body_html` for a caller-provided HTML alternative, or `body_md` to render Markdown as HTML. Messages retain a plain-text alternative; `body` may be omitted when `body_md` is used, and `body_md` and `body_html` cannot be combined.
  - Form `options_source` URLs now preserve existing query parameters when adding the dynamic `search` parameter.
+ - Map coordinates that are not a pair of numbers, like a latitude with no longitude, are now reported in the browser console and skipped, instead of breaking the whole map.
  - Screen readers now announce the title of the modal component instead of an unnamed dialog.
 
 ## v0.45

sqlpage/sqlpage.js

@@ -182,12 +182,19 @@ function sqlpage_map() {
     onLeafletLoad();
   }
   /**
-   *
    * @param {string|undefined} coords
    * @returns {[number, number] | undefined}
    */
   function parseCoords(coords) {
-    return coords?.split(",", 2).map((c) => Number.parseFloat(c));
+    if (!coords) return undefined;
+    const parsed = coords.split(",", 2).map((c) => Number.parseFloat(c));
+    if (parsed.length !== 2 || !parsed.every(Number.isFinite)) {
+      console.error(
+        `Invalid map coordinates: ${JSON.stringify(coords)}. Expected a "latitude,longitude" pair of numbers.`,
+      );
+      return undefined;
+    }
+    return [parsed[0], parsed[1]];
   }
   function onLeafletLoad() {
     is_leaflet_loaded = true;
@@ -230,6 +237,7 @@ function sqlpage_map() {
     const marker = dataset.coords
       ? createMarker(marker_elem, options)
       : createGeoJSONMarker(marker_elem, options);
+    if (!marker) return;
     marker.addTo(map);
     map._sqlpage_markers.push(marker);
     if (marker_elem.textContent.trim()) marker.bindPopup(marker_elem);
@@ -241,6 +249,7 @@ function sqlpage_map() {
   }
   function createMarker(marker_elem, options) {
     const coords = parseCoords(marker_elem.dataset.coords);
+    if (!coords) return undefined;
     const icon_obj = marker_elem.getElementsByClassName("mapicon")[0];
     if (icon_obj) {
       const size =

tests/end-to-end/map-component.spec.ts

@@ -0,0 +1,116 @@
+import { expect, type Page, test } from "@playwright/test";
+
+const BASE = process.env.SQLPAGE_TEST_BASE ?? "http://localhost:8080/";
+
+declare global {
+  function sqlpage_map(): void;
+}
+
+type Marker = { coords?: string; title: string };
+
+const PARIS = "48.85,2.35";
+const PARIS_WITHOUT_ITS_LONGITUDE = "48.85,";
+const NOT_COORDINATES = "somewhere nice";
+
+async function renderMap(
+  page: Page,
+  center: string | null,
+  markers: Marker[] = [],
+) {
+  return page.evaluate(
+    async ({ center, markers }) => {
+      document.getElementById("test-map")?.remove();
+      const container = document.createElement("div");
+      container.id = "test-map";
+      container.className = "leaflet";
+      container.style.height = "200px";
+      container.dataset.zoom = "5";
+      container.dataset.max_zoom = "18";
+      if (center !== null) container.dataset.center = center;
+      container.innerHTML = markers
+        .map(
+          (m) =>
+            `<div class="marker"${m.coords === undefined ? "" : ` data-coords="${m.coords}"`}><h3>${m.title}</h3></div>`,
+        )
+        .join("");
+      container.dataset.preInit = "map";
+      document.body.appendChild(container);
+
+      const errors: string[] = [];
+      const record = (e: ErrorEvent) => errors.push(e.message);
+      window.addEventListener("error", record);
+
+      const logged: string[] = [];
+      const console_error = console.error;
+      console.error = (...args) => logged.push(args.join(" "));
+
+      sqlpage_map();
+      await new Promise((resolve) => setTimeout(resolve, 500));
+
+      console.error = console_error;
+      window.removeEventListener("error", record);
+
+      return {
+        errors,
+        logged,
+        markers: container.querySelectorAll(".leaflet-marker-icon").length,
+        initialized: !!container.querySelector(".leaflet-map-pane"),
+      };
+    },
+    { center, markers },
+  );
+}
+
+test.beforeEach(async ({ page }) => {
+  await page.goto(`${BASE}/documentation.sql?component=map#component`);
+  await page.waitForFunction(() => "L" in window);
+});
+
+test("centers the map on a pair of coordinates", async ({ page }) => {
+  const map = await renderMap(page, PARIS);
+
+  expect(map.errors).toEqual([]);
+  expect(map.logged).toEqual([]);
+  expect(map.initialized).toBe(true);
+});
+
+test("reports a center whose longitude is missing", async ({ page }) => {
+  const map = await renderMap(page, PARIS_WITHOUT_ITS_LONGITUDE);
+
+  expect(map.errors).toEqual([]);
+  expect(map.logged).toEqual([
+    expect.stringContaining(PARIS_WITHOUT_ITS_LONGITUDE),
+  ]);
+  expect(map.initialized).toBe(true);
+});
+
+test("reports a center that is not a pair of numbers", async ({ page }) => {
+  const map = await renderMap(page, NOT_COORDINATES);
+
+  expect(map.errors).toEqual([]);
+  expect(map.logged).toEqual([expect.stringContaining(NOT_COORDINATES)]);
+  expect(map.initialized).toBe(true);
+});
+
+test("draws a marker at a pair of coordinates", async ({ page }) => {
+  const map = await renderMap(page, PARIS, [{ coords: PARIS, title: "Paris" }]);
+
+  expect(map.errors).toEqual([]);
+  expect(map.logged).toEqual([]);
+  expect(map.markers).toBe(1);
+});
+
+test("reports a marker whose longitude is missing, keeping the others", async ({
+  page,
+}) => {
+  const map = await renderMap(page, PARIS, [
+    { coords: PARIS_WITHOUT_ITS_LONGITUDE, title: "Half of Paris" },
+    { coords: PARIS, title: "Paris" },
+  ]);
+
+  expect(map.errors).toEqual([]);
+  expect(map.logged).toEqual([
+    expect.stringContaining(PARIS_WITHOUT_ITS_LONGITUDE),
+  ]);
+  expect(map.markers).toBe(1);
+});