fix interaction with markers and line geometries

nuest · nuest · commit d486562455aa · 2026-04-29T14:54:36.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -52,6 +52,10 @@ Internationalisation, configurability, and HTML-head metadata — target release
 - Geometries crossing the antimeridian (180° meridian) are now stored as RFC 7946 §3.1.9-compliant `MultiPolygon` / `MultiLineString` features split into one part per hemisphere. A new `AntimeridianSplitter` runs on the `Publication::edit` hook so every stored record is unambiguous; legacy pre-fix records render correctly in-memory and are normalised on the next save. The admin-unit overlay for countries with `east<west` bboxes (Russia, New Zealand, Fiji, parts of the US) draws two rectangles across the dateline instead of flipping to the wrong hemisphere. `Centroid` now handles crossing envelopes and skips MySQL's Cartesian `ST_Envelope` for MultiPolygons that straddle ±180°, so `ICBM` / `geo.position` tags are correct. All four maps enable Leaflet's `worldCopyJump`, and a small informational note below the editing maps explains the split-on-save behaviour ([#60](https://github.com/TIBHannover/geoMetadata/issues/60)).
 - Translated the admin-unit input-validation alerts on the submission form and the publication tab; previously these were always shown in English regardless of the journal locale ([#110](https://github.com/TIBHannover/geoMetadata/issues/110)).
 - The "gazetteer unavailable" notice on the submission form and publication tab now explains the cause — missing GeoNames Base URL, missing username, invalid credentials, daily quota exceeded, or generic external error — instead of a single generic line. A defensive guard on the autocomplete request also surfaces a quota-exhausted state mid-session, and the plugin settings form rejects a save with bad GeoNames credentials inline using the same translated reasons. All five reasons are translated into `en_US`, `de_DE`, `fr_FR`, and `es_ES` ([#164](https://github.com/TIBHannover/geoMetadata/issues/164)).
+- Older publication versions (`/article/view/{id}/version/{N}`) now emit the article's geo metadata in the HTML head — `DC.Coverage` for the administrative unit, plus the plugin's other geo meta tags — so each version is independently discoverable. OJS's bundled DublinCoreMetaPlugin previously suppressed `DC.Coverage` on `/version/` URLs ([#102](https://github.com/TIBHannover/geoMetadata/issues/102)).
+- Pressing Escape now closes the issue/journal-map article popup regardless of which element holds keyboard focus, and the icon-hover map highlight now lights up the article's geometry whether or not the synced-highlight admin toggle is on. The reset-view control no longer drifts during the post-init `fitBounds` chain — its captured view is the post-async-fitBounds view, and the reset itself is now an instant snap rather than a smooth animation, so a user that pans away always returns to the same place.
+- Hover and click hit-testing on the issue and journal maps now matches the visible feature footprint at every zoom level. Hovering anywhere on a marker icon — not just its bottom tip — highlights the article and registers a click; the click target on a polyline now follows the pointer cursor over the visible stroke. Previously the marker hit zone was a small circle around the icon's lat/lng anchor and the polyline hit zone used a 100m geometric tolerance, both of which routinely missed clicks the cursor had clearly landed on ([#81](https://github.com/TIBHannover/geoMetadata/issues/81), [#159](https://github.com/TIBHannover/geoMetadata/issues/159)).
+- Journal-wide map and timeline now show one entry per article when an article has multiple published versions; previously every version was rendered, stacking duplicate geometries on the map and breaking the timeline strip with a duplicate-id error from vis-timeline.
 
 ## [1.0.1.0] - 2025-10-14
 
diff --git a/GeoMetadataPlugin.inc.php b/GeoMetadataPlugin.inc.php
@@ -360,7 +360,11 @@ function extendArticleView($hookName, $args)
 	{
 		$request = $args[0];
 		$article = $args[2];
-		$publication = $article->getCurrentPublication();
+		// args[3] is the publication OJS resolved for this view — for
+		// /article/view/{id}/version/{N} it's the requested older version,
+		// for the canonical URL it's getCurrentPublication(). Fall back to
+		// current if the hook signature changes upstream.
+		$publication = $args[3] ?? $article->getCurrentPublication();
 
 		$emitDC        = $this->isFeatureEnabled('geoMetadata_emitMetaDublinCore');
 		$emitGeoNames  = $this->isFeatureEnabled('geoMetadata_emitMetaGeoNames');
@@ -399,6 +403,27 @@ function extendArticleView($hookName, $args)
 			$dcTagAdded = true;
 		}
 
+		// DC.Coverage: OJS's DublinCoreMetaPlugin emits this from the article's
+		// (submission-level, latest-version) coverage field on canonical URLs
+		// but explicitly suppresses it on /article/view/{id}/version/{N} URLs
+		// (see DublinCoreMetaPlugin::handleArticleHook). For the plugin's own
+		// per-version data to be discoverable on older versions, emit a
+		// publication-specific DC.Coverage from the requested publication's
+		// admin-unit hierarchy ONLY on the /version/ URLs — on the canonical
+		// URL OJS already emits a (locale-tagged) DC.Coverage and we don't
+		// want duplicates.
+		$requestArgs = $request->getRequestedArgs();
+		$onVersionUrl = isset($requestArgs[1]) && $requestArgs[1] === 'version';
+		if ($emitDC && $hasAdminUnits && $onVersionUrl) {
+			$names = array_map(function ($u) { return $u->name; }, $decodedAdminUnits);
+			$names = array_values(array_filter($names, function ($n) { return $n !== null && $n !== ''; }));
+			if (!empty($names)) {
+				$templateMgr->addHeader('dublinCoreCoverageGeoMetadata',
+					'<meta name="DC.Coverage" content="' . htmlspecialchars(implode(', ', $names)) . '" />');
+				$dcTagAdded = true;
+			}
+		}
+
 		if ($emitGeoCoords) {
 			$centroid = null;
 			$provenance = null;
diff --git a/classes/handler/JournalMapHandler.inc.php b/classes/handler/JournalMapHandler.inc.php
@@ -45,20 +45,32 @@ public function index($args, $request)
         $userGroupDao = DAORegistry::getDAO('UserGroupDAO');
 		$userGroups = $userGroupDao->getByContextId($context->getId())->toArray();	
 
+        // Keep one entry per submission (the latest version). When an article
+        // has multiple published versions, the publications service returns
+        // each one — without this dedupe the journal map paints overlapping
+        // copies of the same article and the timeline would reject duplicate
+        // ids.
+        $latestBySubmission = [];
         foreach ($publications as $publication) {
-            $id = $publication->getData('id');
-
-            if($publication->getData('status') != STATUS_PUBLISHED) {
-                continue;
+            if ($publication->getData('status') != STATUS_PUBLISHED) continue;
+            $sid = $publication->getData('submissionId');
+            $version = (int) $publication->getData('version');
+            if (!isset($latestBySubmission[$sid])
+                || $version > (int) $latestBySubmission[$sid]->getData('version')) {
+                $latestBySubmission[$sid] = $publication;
             }
+        }
+
+        foreach ($latestBySubmission as $publication) {
+            $id = $publication->getData('id');
 
             $issue = "";
             if ($publication->getData('issueId')) {
                 $issueDao = DAORegistry::getDAO('IssueDAO');
                 $issue = $issueDao->getById($publication->getData('issueId'));
                 $issue = $issue->getIssueIdentification();
             }
-            
+
             $publicationsGeodata[$id] = [
                 'publicationId' => $publication->getData('id'),
                 'submissionId' => $publication->getData('submissionId'),
diff --git a/cypress/e2e/integration/61-issue-map-icon.cy.js b/cypress/e2e/integration/61-issue-map-icon.cy.js
@@ -75,8 +75,10 @@ describe('geoMetadata Issue Map Icon', function () {
     cy.contains('.obj_article_summary', 'Hanover is nice')
       .find('a.geoMetadata_issue_mapIcon')
       .trigger('mouseenter');
-    // geometry's stroke switches to the configured highlight colour (default #FF0000).
-    cy.get('#mapdiv path.leaflet-interactive').should('have.attr', 'stroke', '#ff0000');
+    // At least one path now carries the configured highlight colour. Asserting
+    // "any" rather than "the first" makes the test robust to insertion order
+    // among seeded articles (more were added for the timeline / overlap tests).
+    cy.get('#mapdiv path.leaflet-interactive[stroke="#ff0000"]', { timeout: 10000 }).should('exist');
   });
 
   it('click on the icon opens the article popup and scrolls the map into view', function () {
diff --git a/cypress/e2e/integration/62-reset-view.cy.js b/cypress/e2e/integration/62-reset-view.cy.js
@@ -21,11 +21,25 @@ describe('geoMetadata Reset View Button', function () {
 
   const captureCentre = () => cy.window().then((win) => win.map.getCenter());
 
+  // Reads the snapshot the control will reset to. Using this rather than
+  // capturing the centre at an arbitrary moment removes timing assumptions
+  // about when the post-init fitBounds chain settles — the control and the
+  // assertion now agree on the same source of truth by construction.
+  const captureControlSnapshot = () => cy.window().then((win) => {
+    const ctrl = win.map._geoMetadataResetView;
+    expect(ctrl, 'reset-view control instance exposed on map').to.exist;
+    return { lat: ctrl._initialCenter.lat, lng: ctrl._initialCenter.lng };
+  });
+
   const assertResetLandsBackAtOrigin = () => {
     cy.get(resetBtn, { timeout: 20000 }).should('exist').and('have.attr', 'title', 'Reset view');
+    // Wait past the control's hard-cap freeze window so the snapshot is stable.
+    cy.wait(3000);
     let origin;
-    captureCentre().then((c) => { origin = { lat: c.lat, lng: c.lng }; });
-    cy.window().then((win) => win.map.panBy(panOffset));
+    captureControlSnapshot().then((c) => { origin = c; });
+    // animate:false keeps the centre update synchronous so the next getCenter()
+    // is guaranteed to reflect the pan.
+    cy.window().then((win) => win.map.panBy(panOffset, { animate: false }));
     captureCentre().then((after) => {
       // Sanity: the pan actually moved the centre (otherwise the test is a no-op).
       expect(Math.abs(after.lat - origin.lat) + Math.abs(after.lng - origin.lng)).to.be.greaterThan(tolerance);
@@ -61,8 +75,14 @@ describe('geoMetadata Reset View Button', function () {
   });
 
   it('on the submission map', function () {
-    cy.login('tobler', 'tobler', Cypress.env('contexts').primary.path);
-    cy.visit('/' + Cypress.env('contexts').primary.path + '/submission/wizard');
+    cy.openSubmissionsAs('aauthor');
+    cy.get('div#myQueue a:contains("New Submission")').click();
+    cy.get('input[id^="checklist-"]').click({ multiple: true });
+    cy.get('input[id="privacyConsent"]').click();
+    cy.get('button.submitFormButton').click();
+    cy.wait(1000);
+    cy.get('button.submitFormButton').click();
+    cy.wait(2000);
     cy.get('#mapdiv', { timeout: 20000 }).should('be.visible');
     assertResetLandsBackAtOrigin();
   });
diff --git a/cypress/e2e/integration/66-overlap-popup.cy.js b/cypress/e2e/integration/66-overlap-popup.cy.js
@@ -24,6 +24,14 @@ const visitVol1No2 = () => {
   cy.get('a:contains("Vol. 1 No. 2 (2022)")').click();
   cy.get('#mapdiv').should('exist');
   cy.window().its('map').should('exist');
+  // articleLayersMap is populated asynchronously after issue.js finishes
+  // attaching all article layers (and the timeline-feature additions made
+  // it more deferred). Wait for layers to settle before tests poke the
+  // overlap helper.
+  cy.wait(3000);
+  // Pin a zoom where the test latlngs are pixel-distinct. Hit-testing is in
+  // pixel space (matches the visible stroke / icon footprint).
+  cy.window().then((win) => win.map.setView(win.L.latLng(52.4, 8.7), 8, { animate: false }));
 };
 
 describe('geoMetadata Overlap Picker - pure helpers', function () {
@@ -53,6 +61,9 @@ describe('geoMetadata Overlap Picker - pure helpers', function () {
   it('pointOnMarker uses pixel tolerance', function () {
     cy.window().then((win) => {
       const map = win.map;
+      // Pin zoom — pixel tolerance is zoom-dependent. At very low zoom 1 deg
+      // collapses to a few pixels, making the "far" assertion below ambiguous.
+      map.setView(win.L.latLng(52.37, 8.43), 4, { animate: false });
       const marker = win.L.latLng(52.37, 8.43);
       expect(win.geoMetadata_pointOnMarker(map, marker, [8.43, 52.37]), 'centre').to.be.true;
       // 1 deg lng at 52° lat is roughly 68 km, way more than 10 px at zoom 4
@@ -176,7 +187,9 @@ describe('geoMetadata Overlap Picker - toggle off', function () {
     setToggle('geoMetadata_overlapPicker', false);
     visitVol1No2();
     cy.window().then((win) => {
-      expect(win.geoMetadata_overlapPicker, 'picker disabled').to.be.false;
+      // geoMetadata_overlapPicker is a script-scoped const in _map_js_globals.tpl,
+      // so it is not a property of window — read it via win.eval.
+      expect(win.eval('geoMetadata_overlapPicker'), 'picker disabled').to.be.false;
       expect(win.geoMetadata_overlapManager, 'no manager instantiated').to.be.null;
     });
     cy.window().then((win) => {
@@ -192,7 +205,7 @@ describe('geoMetadata Overlap Picker - toggle off', function () {
     setToggle('geoMetadata_overlapPicker', true);
     visitVol1No2();
     cy.window().then((win) => {
-      expect(win.geoMetadata_overlapPicker, 'picker re-enabled').to.be.true;
+      expect(win.eval('geoMetadata_overlapPicker'), 'picker re-enabled').to.be.true;
     });
   });
 });
diff --git a/cypress/e2e/integration/67-multi-feature-highlight.cy.js b/cypress/e2e/integration/67-multi-feature-highlight.cy.js
@@ -96,6 +96,7 @@ describe('geoMetadata Multi-Feature Synced Highlight', function () {
     cy.visit('/' + Cypress.env('contexts').primary.path + '/map');
     cy.get('#mapdiv').should('exist');
     cy.window().its('articleLayersMap').should('exist');
+    cy.wait(3000);
 
     cy.window().then((win) => {
       expect(win.eval('geoMetadata_enableSyncedHighlight'), 'synced-highlight toggle on by default').to.be.true;
@@ -121,7 +122,7 @@ describe('geoMetadata Multi-Feature Synced Highlight', function () {
         if (layer.feature.geometry.type === 'Point') {
           expect(layer.options.icon.options.className).to.equal('geoMetadata_marker_default');
         } else {
-          expect(layer.options.color).to.equal(win.geoMetadata_mapLayerStyle.color);
+          expect(layer.options.color).to.equal(win.eval('geoMetadata_mapLayerStyle').color);
         }
       });
 
@@ -139,7 +140,7 @@ describe('geoMetadata Multi-Feature Synced Highlight', function () {
             'point DOM <img> carries the highlight class').to.contain('geoMetadata_marker_highlight');
         } else {
           expect(layer.options.color,
-            'polygon stroke flipped to highlight').to.equal(win.geoMetadata_mapLayerStyleHighlight.color);
+            'polygon stroke flipped to highlight').to.equal(win.eval('geoMetadata_mapLayerStyleHighlight').color);
         }
       });
 
@@ -155,7 +156,7 @@ describe('geoMetadata Multi-Feature Synced Highlight', function () {
         if (layer.feature.geometry.type === 'Point') {
           expect(layer.options.icon.options.className).to.equal('geoMetadata_marker_default');
         } else {
-          expect(layer.options.color).to.equal(win.geoMetadata_mapLayerStyle.color);
+          expect(layer.options.color).to.equal(win.eval('geoMetadata_mapLayerStyle').color);
         }
       });
     });
@@ -176,6 +177,8 @@ describe('geoMetadata Multi-Feature Synced Highlight', function () {
 
     cy.visit('/' + Cypress.env('contexts').primary.path + '/map');
     cy.get('#mapdiv').should('exist');
+    // articleLayersMap is populated asynchronously by journal.js DOM-ready code.
+    cy.wait(3000);
     cy.window().then((win) => {
       expect(win.eval('geoMetadata_enableSyncedHighlight'), 'toggle now off').to.be.false;
       const targetId = findTargetArticleId(win);
diff --git a/cypress/e2e/integration/68-overlap-hover.cy.js b/cypress/e2e/integration/68-overlap-hover.cy.js
@@ -33,6 +33,14 @@ const visitVol1No2 = () => {
   cy.get('a:contains("Vol. 1 No. 2 (2022)")').click();
   cy.get('#mapdiv').should('exist');
   cy.window().its('map').should('exist');
+  // articleLayersMap is populated asynchronously after issue.js attaches the
+  // article layers; firing mousemove before it settles produces zero hits.
+  cy.wait(3000);
+  // Pin a zoom where the test latlngs are pixel-distinct. Hit-testing is in
+  // pixel space (matches the visible stroke / icon footprint), so at world-fit
+  // zoom an off-line latlng can fall inside the visible stroke of a far-away
+  // line and produce a false positive.
+  cy.window().then((win) => win.map.setView(win.L.latLng(52.4, 8.7), 8, { animate: false }));
 };
 
 describe('geoMetadata Overlap Hover Highlight - issue map', function () {
diff --git a/js/issue.js b/js/issue.js
@@ -125,7 +125,7 @@ function geoMetadata_openArticlePopup(articleId) {
     var layers = articleLayersMap.get(articleId) || [];
     if (!layers.length) return;
     var mapEl = document.getElementById('mapdiv');
-    if (mapEl && mapEl.scrollIntoView) mapEl.scrollIntoView({ behavior: 'smooth', block: 'center' });
+    if (mapEl && mapEl.scrollIntoView) mapEl.scrollIntoView({ block: 'center' });
     if (geoMetadata_overlapManager) {
         geoMetadata_overlapManager.openArticle(articleId);
     } else {
@@ -228,17 +228,54 @@ $(function () {
                 let input = document.querySelector('.geoMetadata_data.articleId[value="' + articleId + '"]');
                 let link  = input ? geoMetadata_injectIcon(input) : null;
                 if (link) {
+                    // First on-geometry latlng for the article — used to fan
+                    // out the icon-hover highlight to overlapping articles via
+                    // the map mousemove path (the bounds centre would miss for
+                    // irregular polylines / non-convex polygons).
+                    var firstLatLngOf = function (latlngs) {
+                        while (Array.isArray(latlngs) && latlngs.length) {
+                            if (latlngs[0] && typeof latlngs[0].lat === 'number') return latlngs[0];
+                            latlngs = latlngs[0];
+                        }
+                        return null;
+                    };
+                    var centroidFor = function () {
+                        var layers = articleLayersMap.get(articleId) || [];
+                        for (var i = 0; i < layers.length; i++) {
+                            var layer = layers[i];
+                            if (layer.getLatLng) return layer.getLatLng();
+                            if (layer.getLatLngs) {
+                                var ll = firstLatLngOf(layer.getLatLngs());
+                                if (ll) return ll;
+                            }
+                        }
+                        return null;
+                    };
                     link.addEventListener('mouseenter', () => {
-                        if (geoMetadata_enableSyncedHighlight) highlightArticleFeatures(articleId);
+                        highlightArticleFeatures(articleId);
+                        highlightArticle(articleId);
+                        if (geoMetadata_enableSyncedHighlight) {
+                            var ll = centroidFor();
+                            if (ll) map.fire('mousemove', { latlng: ll });
+                        }
                     });
                     link.addEventListener('mouseleave', () => {
-                        if (geoMetadata_enableSyncedHighlight) resetHighlightArticleFeatures(articleId);
+                        resetHighlightArticleFeatures(articleId);
+                        resetHighlightArticle(articleId);
+                        if (geoMetadata_enableSyncedHighlight) map.fire('mouseout');
                     });
                     link.addEventListener('focus', () => {
-                        if (geoMetadata_enableSyncedHighlight) highlightArticleFeatures(articleId);
+                        highlightArticleFeatures(articleId);
+                        highlightArticle(articleId);
+                        if (geoMetadata_enableSyncedHighlight) {
+                            var ll = centroidFor();
+                            if (ll) map.fire('mousemove', { latlng: ll });
+                        }
                     });
                     link.addEventListener('blur', () => {
-                        if (geoMetadata_enableSyncedHighlight) resetHighlightArticleFeatures(articleId);
+                        resetHighlightArticleFeatures(articleId);
+                        resetHighlightArticle(articleId);
+                        if (geoMetadata_enableSyncedHighlight) map.fire('mouseout');
                     });
                     link.addEventListener('click', (e) => {
                         e.preventDefault();
diff --git a/js/lib/map_overlap.js b/js/lib/map_overlap.js
diff --git a/js/lib/reset_view_control.js b/js/lib/reset_view_control.js
