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 <div data-umami-event=...> 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).
This commit is contained in:
+11
-13
@@ -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);
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user