Pick the first non-null target for INP attribution (#421)

* Update onINP.ts * Update src/attribution/onINP.ts Co-authored-by: Barry Pollard <[email protected]> * Apply suggestions from code review Co-authored-by: Barry Pollard <[email protected]> * prettier * Apply suggestions from code review Co-authored-by: Philip Walton <[email protected]> * prettier * Add test for empty target logic --------- Co-authored-by: Barry Pollard <[email protected]> Co-authored-by: Philip Walton <[email protected]> Co-authored-by: Philip Walton <[email protected]>
GoogleChrome · Jan 24, 2024 · 92ea1e5 · 92ea1e5
1 parent 2301de5
commit 92ea1e5
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 16 deletions.
diff --git a/src/attribution/onINP.ts b/src/attribution/onINP.ts
@@ -37,8 +37,17 @@ const attributeINP = (metric: INPMetric): void => {
       );
     })[0];
 
+    // Currently Chrome can return a null target for certain event types
+    // (especially pointer events). As the event target should be the same
+    // for all events in the same interaction, we pick the first non-null one.
+    // TODO: remove when 1367329 is resolved
+    // https://bugs.chromium.org/p/chromium/issues/detail?id=1367329
+    const firstEntryWithTarget = metric.entries.find((entry) => entry.target);
+
     (metric as INPMetricWithAttribution).attribution = {
-      eventTarget: getSelector(longestEntry.target),
+      eventTarget: getSelector(
+        firstEntryWithTarget && firstEntryWithTarget.target,
+      ),
       eventType: longestEntry.name,
       eventTime: longestEntry.startTime,
       eventEntry: longestEntry,

diff --git a/test/e2e/onINP-test.js b/test/e2e/onINP-test.js
@@ -282,6 +282,9 @@ describe('onINP()', async function () {
     const h1 = await $('h1');
     await h1.click();
 
+    // Ensure the interaction completes.
+    await nextFrame();
+
     await stubForwardBack();
     await beaconCountIs(1);
 
@@ -452,9 +455,10 @@ describe('onINP()', async function () {
       assert.equal(eventEntry1.processingStart, clickEntry.processingStart);
 
       await clearBeacons();
-      await setBlockingTime('pointerup', 200);
 
       await stubVisibilityChange('visible');
+      await setBlockingTime('pointerup', 200);
+
       const reset = await $('#reset');
       await reset.click();
 
@@ -525,6 +529,37 @@ describe('onINP()', async function () {
       const [inp2] = await getBeacons();
       assert.equal(inp2.attribution.loadState, 'loading');
     });
+
+    // TODO: remove this test once the following bug is fixed:
+    // https://bugs.chromium.org/p/chromium/issues/detail?id=1367329
+    it('reports the event target from any entry where target is defined', async function () {
+      if (!browserSupportsINP) this.skip();
+
+      await browser.url('/test/inp?attribution=1&mousedown=100&click=50');
+
+      const h1 = await $('h1');
+
+      // Simulate a long click on the h1 to ensure the `click` entry
+      // has a startTime after the `pointerdown` entry.
+      await browser
+        .action('pointer')
+        .move({x: 0, y: 0, origin: h1})
+        .down({button: 0}) // left button
+        .pause(100)
+        .up({button: 0})
+        .perform();
+
+      await stubVisibilityChange('hidden');
+      await beaconCountIs(1);
+
+      const [inp1] = await getBeacons();
+
+      assert.equal(inp1.attribution.eventType, 'pointerdown');
+      // The event target should match the h1, even if the `pointerdown`
+      // entry doesn't contain a target.
+      // See: https://bugs.chromium.org/p/chromium/issues/detail?id=1367329
+      assert.equal(inp1.attribution.eventTarget, 'html>body>main>h1');
+    });
   });
 });
 
@@ -536,12 +571,7 @@ const interactionIDsMatch = (entries) => {
   return entries.every((e) => e.interactionId === entries[0].interactionId);
 };
 
-const setBlockingTime = (event, value) => {
-  return browser.execute(
-    (event, value) => {
-      document.getElementById(`${event}-blocking-time`).value = value;
-    },
-    event,
-    value,
-  );
+const setBlockingTime = async (event, value) => {
+  const input = await $(`#${event}-blocking-time`);
+  await input.setValue(value);
 };
diff --git a/test/views/inp.njk b/test/views/inp.njk
@@ -17,6 +17,14 @@
 
 {% block content %}
   <h1 elementtiming="main-heading">INP Test</h1>
+  <p>
+    <label><input type="number" value="0" id="mousedown-blocking-time">
+    <code>mousedown</code> blocking time</label>
+  </p>
+  <p>
+    <label><input type="number" value="0" id="mouseup-blocking-time">
+    <code>mouseup</code> blocking time</label>
+  </p>
   <p>
     <label><input type="number" value="0" id="pointerdown-blocking-time">
     <code>pointerdown</code> blocking time</label>
@@ -63,15 +71,40 @@
       }
     }
 
-    addEventListener('pointerdown', block, true);
-    addEventListener('pointerup', block, true);
-    addEventListener('keydown', block, true);
-    addEventListener('keyup', block, true);
-    addEventListener('click', block, true);
+    function onInput(event) {
+      const input = event.target;
+      const eventName = input.id.slice(0, input.id.indexOf('-'));
+      if (input.value > 0) {
+        addEventListener(eventName, block, true);
+      } else {
+        removeEventListener(eventName, block, true);
+      }
+    }
+
+    function addBlockingListeners() {
+      const eventNames = [
+        'mousedown',
+        'mouseup',
+        'pointerdown',
+        'pointerup',
+        'keydown',
+        'keyup',
+        'click',
+      ];
+      for (const eventName of eventNames) {
+        const input = document.getElementById(`${eventName}-blocking-time`);
+        input.addEventListener('input', onInput, true);
+        if (input.value > 0) {
+          addEventListener(eventName, block, true);
+        }
+      }
+    }
 
     document.getElementById('reset').addEventListener('click', () => {
       [...document.querySelectorAll('label>input')].forEach((n) => n.value = 0);
     });
+
+    addBlockingListeners();
   </script>
 
   <p><a id="navigate-away" href="https://example.com">Navigate away</a></p>
@@ -80,7 +113,7 @@
 
   <script type="module">
     function jsonifyNode(node) {
-      return node && node.id ? `#${node.id}` : node.nodeName.toLowerCase();
+      return node?.id ? `#${node.id}` : node?.nodeName?.toLowerCase();
     }
 
     const {onINP} = await __testImport('{{ modulePath }}');