From a16da81efdd7c5fdfea9ef47954a7e0c051480d8 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 8 Jan 2024 23:59:43 +0000
Subject: [PATCH 1/5] Fix hashchange events firing twice during every
restoration visit
---
src/core/drive/visit.js | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/core/drive/visit.js b/src/core/drive/visit.js
index 132819507..0669a6909 100644
--- a/src/core/drive/visit.js
+++ b/src/core/drive/visit.js
@@ -348,9 +348,9 @@ export class Visit {
this.scrollToRestoredPosition() || this.scrollToAnchor() || this.view.scrollToTop()
} else {
this.scrollToAnchor() || this.view.scrollToTop()
- }
- if (this.isSamePage) {
- this.delegate.visitScrolledToSamePageLocation(this.view.lastRenderedLocation, this.location)
+ if (this.isSamePage) {
+ this.delegate.visitScrolledToSamePageLocation(this.view.lastRenderedLocation, this.location)
+ }
}
this.scrolled = true
From 9b338f2e94a2de9643155fe2d4678e33107fd158 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 9 Jan 2024 00:00:35 +0000
Subject: [PATCH 2/5] Fix incorrect url passed in hashchange event oldURL
detail
---
src/core/drive/visit.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/core/drive/visit.js b/src/core/drive/visit.js
index 0669a6909..7b68f1d53 100644
--- a/src/core/drive/visit.js
+++ b/src/core/drive/visit.js
@@ -349,7 +349,7 @@ export class Visit {
} else {
this.scrollToAnchor() || this.view.scrollToTop()
if (this.isSamePage) {
- this.delegate.visitScrolledToSamePageLocation(this.view.lastRenderedLocation, this.location)
+ this.delegate.visitScrolledToSamePageLocation(window.location.href, this.location)
}
}
From 9313bc0a80dac3406780c8f85122ff0a3746c584 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 9 Jan 2024 00:05:49 +0000
Subject: [PATCH 3/5] Fix visits sometimes triggered when using anchor links on
same page
---
src/core/drive/navigator.js | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/core/drive/navigator.js b/src/core/drive/navigator.js
index 8d8d3e0be..0cc643d83 100644
--- a/src/core/drive/navigator.js
+++ b/src/core/drive/navigator.js
@@ -129,13 +129,12 @@ export class Navigator {
locationWithActionIsSamePage(location, action) {
const anchor = getAnchor(location)
- const currentAnchor = getAnchor(this.view.lastRenderedLocation)
const isRestorationToTop = action === "restore" && typeof anchor === "undefined"
return (
action !== "replace" &&
getRequestURL(location) === getRequestURL(this.view.lastRenderedLocation) &&
- (isRestorationToTop || (anchor != null && anchor !== currentAnchor))
+ (isRestorationToTop || anchor != null)
)
}
From a8b27c7203fc875a93f9cc1be691a1845023e51b Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 9 Jan 2024 03:37:01 +0000
Subject: [PATCH 4/5] Add test coverage for same-page anchor hashchange events
---
src/tests/fixtures/one.html | 2 +
src/tests/functional/navigation_tests.js | 65 +++++++++++++++++++++++-
src/tests/helpers/page.js | 5 ++
3 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/src/tests/fixtures/one.html b/src/tests/fixtures/one.html
index 3434d3601..7f5dc5083 100644
--- a/src/tests/fixtures/one.html
+++ b/src/tests/fixtures/one.html
@@ -14,6 +14,8 @@ One
An element with an ID
Redirection link
+ Named1
+ Named2
Replaced only the frame
diff --git a/src/tests/functional/navigation_tests.js b/src/tests/functional/navigation_tests.js
index cdbc90e22..34e249e52 100644
--- a/src/tests/functional/navigation_tests.js
+++ b/src/tests/functional/navigation_tests.js
@@ -19,7 +19,8 @@ import {
visitAction,
waitUntilSelector,
waitUntilNoSelector,
- willChangeBody
+ willChangeBody,
+ baseURL
} from "../helpers/page"
test.beforeEach(async ({ page }) => {
@@ -406,6 +407,68 @@ test("same-page anchor visits do not trigger visit events", async ({ page }) =>
}
})
+test("hashchange events on restoration visits between same-page anchors", async ({ page }) => {
+ await page.goto("/src/tests/fixtures/one.html")
+ await page.waitForLoadState()
+
+ await page.click("#named-anchor")
+ await nextBeat()
+
+ await page.click("#named-anchor-two")
+ await nextBeat()
+
+ await page.evaluate(() => {
+ window.hashchangeEvents = []
+
+ window.addEventListener("hashchange", (event) => {
+ window.hashchangeEvents.push({ oldURL: event.oldURL, newURL: event.newURL })
+ })
+ })
+
+ await page.goBack()
+ await nextBeat()
+
+ assert.deepEqual(
+ await page.evaluate("window.hashchangeEvents"),
+ [{ oldURL: baseURL(page) + "#named-anchor-two", newURL: baseURL(page) + "#named-anchor" }]
+ )
+})
+
+test("moving between multiple same-page anchors", async ({ page }) => {
+ await page.click("#same-origin-anchored-link-named")
+ await page.waitForLoadState()
+
+ await page.evaluate(() => {
+ window.hashchangeEvents = []
+ window.visitEvents = []
+
+ window.addEventListener("hashchange", (event) => {
+ window.hashchangeEvents.push({ oldURL: event.oldURL, newURL: event.newURL })
+ })
+ window.addEventListener("turbo:visit", (event) => {
+ window.visitEvents.push(event.detail)
+ })
+ })
+
+ await page.click("#named-anchor-two")
+ await nextBeat()
+
+ assert.deepEqual(
+ await page.evaluate("window.hashchangeEvents"),
+ [{ oldURL: baseURL(page) + "#named-anchor", newURL: baseURL(page) + "#named-anchor-two" }]
+ )
+ await page.evaluate("window.hashchangeEvents = []")
+
+ await page.click("#named-anchor")
+ await nextBeat()
+
+ assert.deepEqual(
+ await page.evaluate("window.hashchangeEvents"),
+ [{ oldURL: baseURL(page) + "#named-anchor-two", newURL: baseURL(page) + "#named-anchor" }]
+ )
+ assert.deepEqual(await page.evaluate("window.visitEvents"), [])
+})
+
test("correct referrer header", async ({ page }) => {
page.click("#headers-link")
await nextBody(page)
diff --git a/src/tests/helpers/page.js b/src/tests/helpers/page.js
index 886941e6a..ed9554768 100644
--- a/src/tests/helpers/page.js
+++ b/src/tests/helpers/page.js
@@ -37,6 +37,11 @@ export function hash(url) {
return hash
}
+export function baseURL(page) {
+ const url = new URL(page.url())
+ return url.origin + url.pathname
+}
+
export async function hasSelector(page, selector) {
return !!(await page.locator(selector).count())
}
From 222afbdfd7756d10a09e6993c0960dabb0a90f3b Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 10 Jan 2024 02:30:05 +0000
Subject: [PATCH 5/5] Clarify/simplify test scenario
---
src/tests/fixtures/one.html | 4 ++--
src/tests/functional/navigation_tests.js | 16 ++++++++--------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/tests/fixtures/one.html b/src/tests/fixtures/one.html
index 7f5dc5083..e268f8cd2 100644
--- a/src/tests/fixtures/one.html
+++ b/src/tests/fixtures/one.html
@@ -14,8 +14,8 @@ One
An element with an ID
Redirection link
- Named1
- Named2
+ Anchor 1
+ Anchor 2
Replaced only the frame
diff --git a/src/tests/functional/navigation_tests.js b/src/tests/functional/navigation_tests.js
index 34e249e52..767b9990a 100644
--- a/src/tests/functional/navigation_tests.js
+++ b/src/tests/functional/navigation_tests.js
@@ -411,10 +411,10 @@ test("hashchange events on restoration visits between same-page anchors", async
await page.goto("/src/tests/fixtures/one.html")
await page.waitForLoadState()
- await page.click("#named-anchor")
+ await page.click("#anchor-one")
await nextBeat()
- await page.click("#named-anchor-two")
+ await page.click("#anchor-two")
await nextBeat()
await page.evaluate(() => {
@@ -430,12 +430,12 @@ test("hashchange events on restoration visits between same-page anchors", async
assert.deepEqual(
await page.evaluate("window.hashchangeEvents"),
- [{ oldURL: baseURL(page) + "#named-anchor-two", newURL: baseURL(page) + "#named-anchor" }]
+ [{ oldURL: baseURL(page) + "#anchor-two", newURL: baseURL(page) + "#anchor-one" }]
)
})
test("moving between multiple same-page anchors", async ({ page }) => {
- await page.click("#same-origin-anchored-link-named")
+ await page.goto("/src/tests/fixtures/one.html#anchor-one")
await page.waitForLoadState()
await page.evaluate(() => {
@@ -450,21 +450,21 @@ test("moving between multiple same-page anchors", async ({ page }) => {
})
})
- await page.click("#named-anchor-two")
+ await page.click("#anchor-two")
await nextBeat()
assert.deepEqual(
await page.evaluate("window.hashchangeEvents"),
- [{ oldURL: baseURL(page) + "#named-anchor", newURL: baseURL(page) + "#named-anchor-two" }]
+ [{ oldURL: baseURL(page) + "#anchor-one", newURL: baseURL(page) + "#anchor-two" }]
)
await page.evaluate("window.hashchangeEvents = []")
- await page.click("#named-anchor")
+ await page.click("#anchor-one")
await nextBeat()
assert.deepEqual(
await page.evaluate("window.hashchangeEvents"),
- [{ oldURL: baseURL(page) + "#named-anchor-two", newURL: baseURL(page) + "#named-anchor" }]
+ [{ oldURL: baseURL(page) + "#anchor-two", newURL: baseURL(page) + "#anchor-one" }]
)
assert.deepEqual(await page.evaluate("window.visitEvents"), [])
})