Refactor NavigateEvent state checks

* Add a PerformSharedChecks() helper for entry point checks that apply to all functions on NavigateEvent (e.g., checking for detached window, whether the event is trusted, etc.) * Add a state machine for tracking interception state in NavigateEvent. Current states are None, Intercepted, Committed, Scrolled, Finished, and transitions must increase. This is hopefully easy to follow than the several independent booleans we are currently using. * Adding the state machine shows that it's possible to scroll before commit by calling scroll() during navigate event dispatch (and in fact, several tests do so). Fix that. Change-Id: I48095b97a3463d6bc145114639dbfc5711822b27 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4262131 Commit-Queue: Nate Chapin <[email protected]> Reviewed-by: Domenic Denicola <[email protected]> Cr-Commit-Position: refs/heads/main@{#1109657}
web-platform-tests · Feb 24, 2023 · 4ec997c · 4ec997c
1 parent 2951f64
commit 4ec997c
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 14 deletions.
diff --git a/navigation-api/focus-reset/focus-reset-timing.html b/navigation-api/focus-reset/focus-reset-timing.html
@@ -0,0 +1,61 @@
+<!doctype html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<body>
+<script>
+promise_test(async t => {
+  navigation.addEventListener("navigate", e => {
+    e.intercept({ focusReset: "after-transition" });
+  }, { once: true });
+
+  const button = document.body.appendChild(document.createElement("button"));
+  const button2 = document.body.appendChild(document.createElement("button"));
+  button2.tabIndex = 0;
+  t.add_cleanup(() => {
+    button.remove();
+    button2.remove();
+  });
+
+  assert_equals(document.activeElement, document.body, "Start on body");
+  button.focus();
+  assert_equals(document.activeElement, button, "focus() worked");
+
+  let navigatesuccess_called = false;
+  navigation.onnavigatesuccess = t.step_func(() => {
+    navigatesuccess_called = true;
+    assert_equals(document.activeElement, document.body, "Focus must be reset before navigatesuccess");
+  });
+
+  await navigation.navigate("#1").finished;
+  assert_true(navigatesuccess_called);
+}, "Focus should be reset before navigatesuccess");
+
+promise_test(async t => {
+  navigation.addEventListener("navigate", e => {
+    e.intercept({ handler: () => Promise.reject(),
+                  focusReset: "after-transition" });
+  }, { once: true });
+
+  const button = document.body.appendChild(document.createElement("button"));
+  const button2 = document.body.appendChild(document.createElement("button"));
+  button2.tabIndex = 0;
+  t.add_cleanup(() => {
+    button.remove();
+    button2.remove();
+  });
+
+  assert_equals(document.activeElement, document.body, "Start on body");
+  button.focus();
+  assert_equals(document.activeElement, button, "focus() worked");
+
+  let navigateerror_called = false;
+  navigation.onnavigateerror = t.step_func(() => {
+    navigateerror_called = true;
+    assert_equals(document.activeElement, document.body, "Focus must be reset before navigateerror");
+  });
+
+  await promise_rejects_exactly(t, undefined, navigation.navigate("#2").finished);
+  assert_true(navigateerror_called);
+}, "Focus should be reset before navigateerror");
+</script>
+</body>
diff --git a/navigation-api/scroll-behavior/after-transition-explicit-scroll.html b/navigation-api/scroll-behavior/after-transition-explicit-scroll.html
@@ -13,10 +13,14 @@
   await navigation.navigate("#frag").finished;
   assert_not_equals(window.scrollY, 0);
   navigation.onnavigate = t.step_func(e => {
-    e.intercept({ scroll: "after-transition" });
-    assert_not_equals(window.scrollY, 0);
-    e.scroll();
-    assert_equals(window.scrollY, 0);
+    e.intercept({
+      scroll: "after-transition",
+      handler: t.step_func(() => {
+        assert_not_equals(window.scrollY, 0);
+        e.scroll();
+        assert_equals(window.scrollY, 0);
+      })
+    });
   });
   await navigation.back().finished;
   assert_equals(window.scrollY, 0);

diff --git a/navigation-api/scroll-behavior/manual-immediate-scroll.html b/navigation-api/scroll-behavior/manual-immediate-scroll.html
@@ -14,11 +14,11 @@
   assert_not_equals(window.scrollY, 0);
   navigation.onnavigate = e => {
     e.intercept({ scroll: "manual" });
-    e.scroll();
-    assert_equals(window.scrollY, 0);
+    assert_throws_dom("InvalidStateError", () => e.scroll());
+    assert_not_equals(window.scrollY, 0);
   }
   await navigation.back().finished;
-  assert_equals(window.scrollY, 0);
-}, "scroll: scroll() should work inside a navigate event handler");
+  assert_not_equals(window.scrollY, 0);
+}, "scroll: scroll() should not work inside a navigate event handler");
 </script>
 </body>
diff --git a/navigation-api/scroll-behavior/manual-scroll-repeated.html b/navigation-api/scroll-behavior/manual-scroll-repeated.html
@@ -12,12 +12,16 @@
   assert_equals(window.scrollY, 0);
   await navigation.navigate("#frag").finished;
   assert_not_equals(window.scrollY, 0);
-  navigation.onnavigate = t.step_func(e => {
-    e.intercept({ scroll: "manual" });
-    e.scroll();
-    assert_equals(window.scrollY, 0);
-    assert_throws_dom("InvalidStateError", () => e.scroll());
-  });
+  navigation.onnavigate = e => {
+    e.intercept({
+      scroll: "manual",
+      handler: t.step_func(() => {
+        e.scroll();
+        assert_equals(window.scrollY, 0);
+        assert_throws_dom("InvalidStateError", () => e.scroll());
+      })
+    });
+  };
   await navigation.back().finished;
   assert_equals(window.scrollY, 0);
 }, "scroll: scroll() should throw if called a second time");

diff --git a/navigation-api/scroll-behavior/scroll-after-preventDefault.html b/navigation-api/scroll-behavior/scroll-after-preventDefault.html
@@ -0,0 +1,21 @@
+<!doctype html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<body>
+<div style="height: 1000px; width: 1000px;"></div>
+<div id="frag"></div>
+<script>
+promise_test(async t => {
+  // Wait for after the load event so that the navigation doesn't get converted
+  // into a replace navigation.
+  await new Promise(resolve => window.onload = () => t.step_timeout(resolve, 0));
+
+  navigation.addEventListener("navigate", t.step_func(e => {
+    e.intercept();
+    e.preventDefault();
+    assert_throws_dom("InvalidStateError", () => e.scroll());
+  }), { once : true });
+  await promise_rejects_dom(t, "AbortError", navigation.navigate("#frag").finished);
+}, "scroll: scroll() should throw after preventDefault");
+</script>
+</body>
diff --git a/navigation-api/scroll-behavior/scroll-on-synthetic-event.html b/navigation-api/scroll-behavior/scroll-on-synthetic-event.html
@@ -0,0 +1,19 @@
+<!doctype html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<body>
+<script>
+promise_test(async t => {
+  // We need to grab an NavigationDestination to construct the event.
+  navigation.onnavigate = t.step_func_done(e => {
+    const event = new NavigateEvent("navigate", {
+      destination: e.destination,
+      signal: (new AbortController()).signal
+    });
+
+    assert_throws_dom("SecurityError", () => event.scroll());
+  });
+  history.pushState(1, null, "#1");
+}, "scroll: scroll() should throw if invoked on a synthetic event.");
+</script>
+</body>