Fix experimental pageleaves script (SPAs) (#4667)

* bugfix: make sure to send pageleave with current URL

* another bugfix: avoid triggering pageleaves in SPAs unless listeningPageLeave is true

* revert behavioural change in manual extension

* test manual script extension

tracker/src/plausible.js

@@ -31,6 +31,11 @@
   {{#if pageleave}}
   // :NOTE: Tracking pageleave events is currently experimental.
 
+  // Keeps track of the URL to be sent in the pageleave event payload.
+  // Should get updated on pageviews triggered manually with a custom
+  // URL, and on SPA navigation.
+  var currentPageLeaveURL = location.href
+
   // Multiple pageviews might be sent by the same script when the page
   // uses client-side routing (e.g. hash or history-based). This flag
   // prevents registering multiple listeners in those cases.
@@ -43,7 +48,7 @@
   // flag prevents sending multiple pageleaves in those cases.
   var pageLeaveSending = false
 
-  function triggerPageLeave(url) {
+  function triggerPageLeave() {
     if (pageLeaveSending) {return}
     pageLeaveSending = true
     setTimeout(function () {pageLeaveSending = false}, 500)
@@ -51,7 +56,7 @@
     var payload = {
       n: 'pageleave',
       d: dataDomain,
-      u: url,
+      u: currentPageLeaveURL,
     }
 
     {{#if hash}}
@@ -64,19 +69,17 @@
     }
   }
 
-  function registerPageLeaveListener(url) {
-    if (listeningPageLeave) { return }
-
-    window.addEventListener('pagehide', function () {
-      triggerPageLeave(url)
-    })
-
-    listeningPageLeave = true
+  function registerPageLeaveListener() {
+    if (!listeningPageLeave) {
+      window.addEventListener('pagehide', triggerPageLeave)
+      listeningPageLeave = true
+    }
   }
   {{/if}}
 
-
   function trigger(eventName, options) {
+    var isPageview = eventName === 'pageview'
+
     {{#unless local}}
     if (/^localhost$|^127(\.[0-9]+){0,2}\.[0-9]+$|^\[::1?\]$/.test(location.hostname) || location.protocol === 'file:') {
       return onIgnoredEvent('localhost', options)
@@ -96,7 +99,7 @@
     var dataIncludeAttr = scriptEl && scriptEl.getAttribute('data-include')
     var dataExcludeAttr = scriptEl && scriptEl.getAttribute('data-exclude')
 
-    if (eventName === 'pageview') {
+    if (isPageview) {
       var isIncluded = !dataIncludeAttr || (dataIncludeAttr && dataIncludeAttr.split(',').some(pathMatches))
       var isExcluded = dataExcludeAttr && dataExcludeAttr.split(',').some(pathMatches)
 
@@ -116,11 +119,19 @@
 
     var payload = {}
     payload.n = eventName
+
     {{#if manual}}
-    payload.u = options && options.u ? options.u : location.href
+    var customURL = options && options.u
+
+    {{#if pageleave}}
+    isPageview && customURL && (currentPageLeaveURL = customURL)
+    {{/if}}
+
+    payload.u = customURL ? customURL : location.href
     {{else}}
     payload.u = location.href
     {{/if}}
+
     payload.d = dataDomain
     payload.r = document.referrer || null
     if (options && options.meta) {
@@ -164,8 +175,8 @@
     request.onreadystatechange = function() {
       if (request.readyState === 4) {
         {{#if pageleave}}
-        if (eventName === 'pageview') {
-          registerPageLeaveListener(payload.u)
+        if (isPageview) {
+          registerPageLeaveListener()
         }
         {{/if}}
         options && options.callback && options.callback({status: request.status})
@@ -182,22 +193,16 @@
   {{#unless manual}}
     var lastPage;
 
-    {{#if pageleave}}
-    var lastUrl = location.href
-
-    function pageLeaveSPA() {
-      triggerPageLeave(lastUrl);
-      lastUrl = location.href;
-    }
-    {{/if}}
-
     function page(isSPANavigation) {
       {{#unless hash}}
       if (lastPage === location.pathname) return;
       {{/unless}}
       
       {{#if pageleave}}
-      if (isSPANavigation) {pageLeaveSPA()}
+      if (isSPANavigation && listeningPageLeave) {
+        triggerPageLeave();
+        currentPageLeaveURL = location.href;
+      }
       {{/if}}
 
       lastPage = location.pathname

tracker/test/fixtures/manual.html

@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<html lang="en">
+
+<head>
+  <meta charset="UTF-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <meta http-equiv="X-UA-Compatible" content="ie=edge">
+  <title>Plausible Playwright tests</title>
+  <script defer src="/tracker/js/plausible.local.manual.js"></script>
+</head>
+
+<body>
+  <button id="pageview-trigger">
+    Triggers a pageview with default URL (location.href)
+  </button>
+  <button id="pageview-trigger-custom-url">
+    Triggers a pageview with custom URL
+  </button>
+  <button id="custom-event-trigger">
+    Triggers a custom event with default URL (location.href)
+  </button>
+  <button id="custom-event-trigger-custom-url">
+    Triggers a custom event with custom URL
+  </button>
+
+  <script>
+    document.addEventListener('click', (e) => {
+      if (e.target.id === 'pageview-trigger') {
+        window.plausible('pageview') 
+      }
+      if (e.target.id === 'pageview-trigger-custom-url') {
+        window.plausible('pageview', {u: 'https://example.com/custom/location'})
+      }
+      if (e.target.id === 'custom-event-trigger') {
+        window.plausible('CustomEvent') 
+      }
+      if (e.target.id === 'custom-event-trigger-custom-url') {
+        window.plausible('CustomEvent', {u: 'https://example.com/custom/location'}) 
+      }
+    })
+  </script>
+</body>
+
+</html>

tracker/test/manual.spec.js

@@ -0,0 +1,35 @@
+const { mockRequest } = require('./support/test-utils')
+const { expect, test } = require('@playwright/test');
+const { LOCAL_SERVER_ADDR } = require('./support/server');
+
+async function clickPageElementAndExpectEventRequest(page, buttonId, expectedBodyParams) {
+  const plausibleRequestMock = mockRequest(page, '/api/event')
+  await page.click(buttonId)
+  const plausibleRequest = await plausibleRequestMock;
+
+  expect(plausibleRequest.url()).toContain('/api/event')
+
+  const body = plausibleRequest.postDataJSON()
+
+  Object.keys(expectedBodyParams).forEach((key) => {
+    expect(body[key]).toEqual(expectedBodyParams[key])
+  })
+}
+
+test.describe('manual extension', () => {
+  test('can trigger custom events with and without a custom URL if pageview was sent with the default URL', async ({ page }) => {
+    await page.goto('/manual.html');
+
+    await clickPageElementAndExpectEventRequest(page, '#pageview-trigger', {n: 'pageview', u: `${LOCAL_SERVER_ADDR}/manual.html`})
+    await clickPageElementAndExpectEventRequest(page, '#custom-event-trigger', {n: 'CustomEvent', u: `${LOCAL_SERVER_ADDR}/manual.html`})
+    await clickPageElementAndExpectEventRequest(page, '#custom-event-trigger-custom-url', {n: 'CustomEvent', u: `https://example.com/custom/location`})
+  });
+
+  test('can trigger custom events with and without a custom URL if pageview was sent with a custom URL', async ({ page }) => {
+    await page.goto('/manual.html');
+
+    await clickPageElementAndExpectEventRequest(page, '#pageview-trigger-custom-url', {n: 'pageview', u: `https://example.com/custom/location`})
+    await clickPageElementAndExpectEventRequest(page, '#custom-event-trigger', {n: 'CustomEvent', u: `${LOCAL_SERVER_ADDR}/manual.html`})
+    await clickPageElementAndExpectEventRequest(page, '#custom-event-trigger-custom-url', {n: 'CustomEvent', u: `https://example.com/custom/location`})
+  });
+});