Bug 1636453 - [cdp] Move Page.navigatedWithinDocument to hashChange r=webdriver-reviewers,whimboo

Depends on D146984

This is needed to be forward compatible with new versions of puppeteer
The navigatedWithinDocument event should only have been emitted for same
document navigation (either hashchange or history navigation), but it was
emitted on load.

Differential Revision: https://phabricator.services.mozilla.com/D146985

remote/cdp/domains/content/Page.jsm

@@ -76,6 +76,10 @@ class Page extends ContentProcessDomain {
       this.chromeEventHandler.addEventListener("DOMContentLoaded", this, {
         mozSystemGroup: true,
       });
+      this.chromeEventHandler.addEventListener("hashchange", this, {
+        mozSystemGroup: true,
+        capture: true,
+      });
       this.chromeEventHandler.addEventListener("load", this, {
         mozSystemGroup: true,
         capture: true,
@@ -111,6 +115,10 @@ class Page extends ContentProcessDomain {
       this.chromeEventHandler.removeEventListener("DOMContentLoaded", this, {
         mozSystemGroup: true,
       });
+      this.chromeEventHandler.removeEventListener("hashchange", this, {
+        mozSystemGroup: true,
+        capture: true,
+      });
       this.chromeEventHandler.removeEventListener("load", this, {
         mozSystemGroup: true,
         capture: true,
@@ -332,10 +340,14 @@ class Page extends ContentProcessDomain {
 
   handleEvent({ type, target }) {
     const timestamp = Date.now() / 1000;
-    const frameId = target.defaultView.docShell.browsingContext.id;
-    const isFrame = !!target.defaultView.docShell.browsingContext.parent;
+
+    // Some events such as "hashchange" use the window as the target, while
+    // others have a document.
+    const win = Window.isInstance(target) ? target : target.defaultView;
+    const frameId = win.docShell.browsingContext.id;
+    const isFrame = !!win.docShell.browsingContext.parent;
     const loaderId = this.frameIdToLoaderId.get(frameId);
-    const url = target.location.href;
+    const url = win.location.href;
 
     switch (type) {
       case "DOMContentLoaded":
@@ -350,6 +362,13 @@ class Page extends ContentProcessDomain {
         );
         break;
 
+      case "hashchange":
+        this.emit("Page.navigatedWithinDocument", {
+          frameId: frameId.toString(),
+          url,
+        });
+        break;
+
       case "pagehide":
         // Maybe better to bound to "unload" once we can register for this event
         this.emit("Page.frameStartedLoading", { frameId: frameId.toString() });
@@ -362,12 +381,6 @@ class Page extends ContentProcessDomain {
         }
         this.emitLifecycleEvent(frameId, loaderId, "load", timestamp);
 
-        // Todo: Only to be emitted for hashchange events (bug 1636453)
-        this.emit("Page.navigatedWithinDocument", {
-          frameId: frameId.toString(),
-          url,
-        });
-
         // XXX this should most likely be sent differently
         this.emit("Page.frameStoppedLoading", { frameId: frameId.toString() });
         break;

remote/cdp/test/browser/browser_cdp.js

@@ -43,7 +43,7 @@ add_task(async function testCDP({ client }) {
   Assert.deepEqual(result, {}, "Got expected result value");
 
   const frameStoppedLoading = Page.frameStoppedLoading();
-  const navigatedWithinDocument = Page.navigatedWithinDocument();
+  const frameNavigated = Page.frameNavigated();
   const loadEventFired = Page.loadEventFired();
   await Page.navigate({
     url: toDataURL(`<script>console.log("foo")</script>`),
@@ -56,6 +56,6 @@ add_task(async function testCDP({ client }) {
   await frameStoppedLoading;
   info("`Page.frameStoppedLoading` fired");
 
-  await navigatedWithinDocument;
-  info("`Page.navigatedWithinDocument` fired");
+  await frameNavigated;
+  info("`Page.frameNavigated` fired");
 });

remote/cdp/test/browser/page/browser.ini

@@ -40,6 +40,7 @@ https_first_disabled = true
 [browser_loadEventFired.js]
 [browser_navigate.js]
 https_first_disabled = true
+[browser_navigatedWithinDocument.js]
 [browser_navigateToHistoryEntry.js]
 [browser_navigationEvents.js]
 [browser_printToPDF.js]

remote/cdp/test/browser/page/browser_navigatedWithinDocument.js

@@ -0,0 +1,133 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+add_task(async function noEventWhenPageDomainDisabled({ client }) {
+  await loadURL(PAGE_URL);
+  await runNavigatedWithinDocumentTest(client, 0, async () => {
+    info("Navigate to the '#hash' anchor in the page");
+    await navigateToAnchor(PAGE_URL, "hash");
+  });
+});
+
+add_task(async function noEventAfterPageDomainDisabled({ client }) {
+  const { Page } = client;
+
+  await Page.enable();
+  await Page.disable();
+
+  await loadURL(PAGE_URL);
+
+  await runNavigatedWithinDocumentTest(client, 0, async () => {
+    info("Navigate to the '#hash' anchor in the page");
+    await navigateToAnchor(PAGE_URL, "hash");
+  });
+});
+
+add_task(async function eventWhenNavigatingToHash({ client }) {
+  const { Page } = client;
+
+  await Page.enable();
+
+  await loadURL(PAGE_URL);
+
+  await runNavigatedWithinDocumentTest(client, 1, async () => {
+    info("Navigate to the '#hash' anchor in the page");
+    await navigateToAnchor(PAGE_URL, "hash");
+  });
+});
+
+add_task(async function eventWhenNavigatingToDifferentHash({ client }) {
+  const { Page } = client;
+
+  await Page.enable();
+
+  await navigateToAnchor(PAGE_URL, "hash");
+
+  await runNavigatedWithinDocumentTest(client, 1, async () => {
+    info("Navigate to the '#hash' anchor in the page");
+    await navigateToAnchor(PAGE_URL, "other-hash");
+  });
+});
+
+add_task(async function eventWhenNavigatingToHashInFrames({ client }) {
+  const { Page } = client;
+
+  await Page.enable();
+
+  await loadURL(FRAMESET_NESTED_URL);
+
+  await runNavigatedWithinDocumentTest(client, 1, async () => {
+    info("Navigate to the '#hash' anchor in the first iframe");
+    await SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
+      const iframe = content.frames[0];
+      const baseUrl = iframe.location.href;
+      iframe.location.href = baseUrl + "#hash-first-frame";
+    });
+  });
+
+  await runNavigatedWithinDocumentTest(client, 2, async () => {
+    info("Navigate to the '#hash' anchor in the nested iframes");
+    await SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
+      const nestedFrame1 = content.frames[0].frames[0];
+      const baseUrl1 = nestedFrame1.location.href;
+      nestedFrame1.location.href = baseUrl1 + "#hash-nested-frame-1";
+      const nestedFrame2 = content.frames[0].frames[0];
+      const baseUrl2 = nestedFrame2.location.href;
+      nestedFrame2.location.href = baseUrl2 + "#hash-nested-frame-2";
+    });
+  });
+});
+
+async function runNavigatedWithinDocumentTest(
+  client,
+  expectedEventCount,
+  callback
+) {
+  const { Page } = client;
+
+  const NAVIGATED = "Page.navigatedWithinDocument";
+
+  const history = new RecordEvents(expectedEventCount);
+  history.addRecorder({
+    event: Page.navigatedWithinDocument,
+    eventName: NAVIGATED,
+    messageFn: payload => {
+      return `Received ${NAVIGATED} for frame id ${payload.frameId}`;
+    },
+  });
+
+  await callback();
+
+  const navigatedWithinDocumentEvents = await history.record();
+
+  is(
+    navigatedWithinDocumentEvents.length,
+    expectedEventCount,
+    "Got expected amount of navigatedWithinDocument events"
+  );
+  if (expectedEventCount == 0) {
+    return;
+  }
+
+  const frames = await getFlattenedFrameTree(client);
+
+  navigatedWithinDocumentEvents.forEach(({ payload }) => {
+    const { frameId, url } = payload;
+
+    const frame = frames.get(frameId);
+    ok(frame, "Returned a valid frame id");
+    is(url, frame.url, "Returned the expectedUrl");
+  });
+}
+
+function navigateToAnchor(baseUrl, hash) {
+  const url = `${baseUrl}#${hash}`;
+  const onLocationChange = BrowserTestUtils.waitForLocationChange(
+    gBrowser,
+    url
+  );
+  BrowserTestUtils.loadURI(gBrowser.selectedBrowser, url);
+  return onLocationChange;
+}

remote/cdp/test/browser/page/browser_navigationEvents.js

@@ -37,7 +37,6 @@ add_task(async function pageWithoutFrame({ client }) {
     recordPromise("frameNavigated", Page.frameNavigated());
     recordPromise("domContentEventFired", Page.domContentEventFired());
     recordPromise("loadEventFired", Page.loadEventFired());
-    recordPromise("navigatedWithinDocument", Page.navigatedWithinDocument());
     recordPromise("frameStoppedLoading", Page.frameStoppedLoading());
   }
 
@@ -125,7 +124,6 @@ async function assertNavigationEvents({ url, frameId }) {
     "frameNavigated",
     "domContentEventFired",
     "loadEventFired",
-    "navigatedWithinDocument",
     "frameStoppedLoading",
   ];
   Assert.deepEqual(
@@ -155,18 +153,6 @@ async function assertNavigationEvents({ url, frameId }) {
   );
   is(frameNavigated.frame.url, url, "frameNavigated url is the right one");
 
-  const navigatedWithinDocument = resolutions.get("navigatedWithinDocument");
-  is(
-    navigatedWithinDocument.frameId,
-    frameId,
-    "navigatedWithinDocument frameId is the same one"
-  );
-  is(
-    navigatedWithinDocument.url,
-    url,
-    "navigatedWithinDocument url is the same one"
-  );
-
   const frameStoppedLoading = resolutions.get("frameStoppedLoading");
   is(
     frameStoppedLoading.frameId,

remote/test/puppeteer-expected.json

@@ -636,10 +636,10 @@
     "PASS"
   ],
   "Frame specs Frame Management should detach child frames on navigation (frame.spec.ts)": [
-    "FAIL"
+    "PASS"
   ],
   "Frame specs Frame Management should support framesets (frame.spec.ts)": [
-    "FAIL"
+    "PASS"
   ],
   "Frame specs Frame Management should report frame from-inside shadow DOM (frame.spec.ts)": [
     "PASS"
@@ -1110,13 +1110,13 @@
     "FAIL"
   ],
   "navigation Page.waitForNavigation should work (navigation.spec.ts)": [
-    "PASS", "FAIL"
+    "PASS"
   ],
   "navigation Page.waitForNavigation should work with both domcontentloaded and load (navigation.spec.ts)": [
     "PASS"
   ],
   "navigation Page.waitForNavigation should work with clicking on anchor links (navigation.spec.ts)": [
-    "TIMEOUT", "FAIL"
+    "PASS"
   ],
   "navigation Page.waitForNavigation should work with history.pushState() (navigation.spec.ts)": [
     "FAIL", "TIMEOUT"