Add url-changed to WebTelemetry (#1865)

* DRAFT: Add urlChanged to WebTelemetry

Uses JK's suggestion to get at the event type.

If I add this to `urlChanged` (in web-telemetry.js) I see that neither of these are true, which confuses me.  It may be that we need a check here, or perhaps this is working because the WebTelemetry feature is enabled and that's by design.

```js
console.log('WebTelemetry: URL changed', navigationType, this.getFeatureSettingEnabled('urlChanged'), this.getFeatureSettingEnabled('listenForUrlChanges'));
```

* Apply suggestions from code review

Co-authored-by: Jonathan Kingston <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jonathan Kingston <[email protected]>

* Update injected/src/url-change.js

* lint-fix

---------

Co-authored-by: Jonathan Kingston <[email protected]>

Author: petevb

injected/src/content-scope-features.js

@@ -77,12 +77,12 @@ export async function init(args) {
             featureInstance.callInit(args);
             // Either listenForUrlChanges or urlChanged ensures the feature listens.
             if (featureInstance.listenForUrlChanges || featureInstance.urlChanged) {
-                registerForURLChanges(() => {
+                registerForURLChanges((navigationType) => {
                     // The rationale for the two separate call here is to ensure that
                     // extensions to the class don't need to call super.urlChanged()
                     featureInstance.recomputeSiteObject();
                     // Called if the feature instance has a urlChanged method
-                    featureInstance?.urlChanged();
+                    featureInstance?.urlChanged(navigationType);
                 });
             }
         }

injected/src/features/web-telemetry.js

@@ -1,8 +1,11 @@
 import ContentFeature from '../content-feature.js';
 
 const MSG_VIDEO_PLAYBACK = 'video-playback';
+const MSG_URL_CHANGED = 'url-changed';
 
 export class WebTelemetry extends ContentFeature {
+    listenForUrlChanges = true;
+
     constructor(featureName, importConfig, args) {
         super(featureName, importConfig, args);
         this.seenVideoElements = new WeakSet();
@@ -15,6 +18,12 @@ export class WebTelemetry extends ContentFeature {
         }
     }
 
+    urlChanged(navigationType = 'unknown') {
+        if (this.getFeatureSettingEnabled('urlChanged')) {
+            this.fireTelemetryForUrlChanged(navigationType);
+        }
+    }
+
     getVideoUrl(video) {
         // Try to get the video URL from various sources
         if (video.src) {
@@ -31,6 +40,13 @@ export class WebTelemetry extends ContentFeature {
         return null;
     }
 
+    fireTelemetryForUrlChanged(navigationType) {
+        this.messaging.notify(MSG_URL_CHANGED, {
+            url: window.location.href,
+            navigationType,
+        });
+    }
+
     fireTelemetryForVideo(video) {
         const videoUrl = this.getVideoUrl(video);
         if (this.seenVideoUrls.has(videoUrl)) {

injected/src/url-change.js

@@ -13,21 +13,30 @@ export function registerForURLChanges(listener) {
     urlChangeListeners.add(listener);
 }
 
-function handleURLChange() {
+function handleURLChange(navigationType = 'unknown') {
     for (const listener of urlChangeListeners) {
-        listener();
+        listener(navigationType);
     }
 }
 
 function listenForURLChanges() {
     const urlChangedInstance = new ContentFeature('urlChanged', {}, {});
+    // if the browser supports the navigation API, use that to listen for URL changes
     if ('navigation' in globalThis && 'addEventListener' in globalThis.navigation) {
-        // if the browser supports the navigation API, we can use that to listen for URL changes
-        // Listening to navigatesuccess instead of navigate to ensure the navigation is committed.
-        globalThis.navigation.addEventListener('navigatesuccess', () => {
-            handleURLChange();
+        // We listen to `navigatesuccess` instead of `navigate` to ensure the navigation is committed.
+        // But, `navigatesuccess` does not provide the navigationType, so we capture it at `navigate` time
+        // then look it up later.  This allows consumers to filter on navigationType.
+        // WeakMap ensures we don't hold onto the event.target longer than necessary and can be freed.
+        const navigations = new WeakMap();
+        globalThis.navigation.addEventListener('navigate', (event) => {
+            navigations.set(event.target, event.navigationType);
         });
-        // Exit early if the navigation API is supported
+        globalThis.navigation.addEventListener('navigatesuccess', (event) => {
+            const navigationType = navigations.get(event.target) || 'unknown';
+            handleURLChange(navigationType);
+            navigations.delete(event.target);
+        });
+        // Exit early if the navigation API is supported, i.e. history proxy and popState listener aren't created.
         return;
     }
     if (isBeingFramed()) {
@@ -39,13 +48,14 @@ function listenForURLChanges() {
     const historyMethodProxy = new DDGProxy(urlChangedInstance, History.prototype, 'pushState', {
         apply(target, thisArg, args) {
             const changeResult = DDGReflect.apply(target, thisArg, args);
-            handleURLChange();
+            console.log('pushstate event');
+            handleURLChange('push');
             return changeResult;
         },
     });
     historyMethodProxy.overload();
     // listen for popstate events in order to run on back/forward navigations
     window.addEventListener('popstate', () => {
-        handleURLChange();
+        handleURLChange('popState');
     });
 }