From dcf1b8b8b524e4a31b559e5992cc5e3ae1e47174 Mon Sep 17 00:00:00 2001 From: Stanislaw <56738034+anvme@users.noreply.github.com> Date: Thu, 7 May 2026 16:04:16 +0200 Subject: [PATCH] fix(tracker): track clicks on annotated containers and gracefully handle invalid pushState URLs Three bugs in src/tracker/index.js, all empirically reproduced. 1. handleClicks() missed clicks on container elements: closest('a,button') could not find a non-anchor ancestor with data-umami-event, so a click on any descendant of
went untracked. Reworked to closest([data-umami-event]) so any annotated ancestor matches. 2. handlePush() called new URL(url, location.href) outside normalize()'s try/catch, so a host page calling history.pushState({}, '', invalidUrl) would have umami's wrapper throw a TypeError into the host's router. normalize(url) already handles base resolution and catches parse errors. 3. The history hook ran the umami callback BEFORE native pushState, so a failed native call (SecurityError on invalid URL, etc.) would still mutate currentUrl/currentRef and schedule a phantom pageview. Run native first; if it throws, the callback never fires and tracker state stays consistent. Bug 2 verified: 4/10 representative click scenarios missed before (span inside div, deep span inside div, a with no href, button inside a), all 10/10 tracked after. Bug 1+3 verified: pushState({}, '', invalidUrl) now leaves tracker state unchanged (currentUrl unchanged, no phantom pageview). --- src/tracker/index.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/tracker/index.js b/src/tracker/index.js index 521a1f717..12ddb3abc 100644 --- a/src/tracker/index.js +++ b/src/tracker/index.js @@ -89,7 +89,7 @@ } currentRef = currentUrl; - currentUrl = normalize(new URL(url, location.href).toString()); + currentUrl = normalize(url); if (currentUrl !== currentRef && autoPageview) { setTimeout(track, delayDuration); @@ -100,8 +100,9 @@ const hook = (_this, method, callback) => { const orig = _this[method]; return (...args) => { + const result = orig.apply(_this, args); callback.apply(null, args); - return orig.apply(_this, args); + return result; }; }; @@ -123,18 +124,13 @@ return track(eventName, eventData); } }; - const onClick = async e => { + const onClick = e => { const el = e.target; - const parentElement = el.closest('a,button'); - if (!parentElement) return trackElement(el); + const eventEl = el.closest(`[${eventNameAttribute}]`); + if (!eventEl) return; - const { href, target } = parentElement; - if (!parentElement.getAttribute(eventNameAttribute)) return; - - if (parentElement.tagName === 'BUTTON') { - return trackElement(parentElement); - } - if (parentElement.tagName === 'A' && href) { + if (eventEl.tagName === 'A' && eventEl.href) { + const { href, target } = eventEl; const external = target === '_blank' || e.ctrlKey || @@ -142,12 +138,14 @@ e.metaKey || (e.button && e.button === 1); if (!external) e.preventDefault(); - return trackElement(parentElement).then(() => { + return trackElement(eventEl).then(() => { if (!external) { (target === '_top' ? top.location : location).href = href; } }); } + + return trackElement(eventEl); }; document.addEventListener('click', onClick, true); };