diff --git a/src/core/drive/history.js b/src/core/drive/history.js index 45015b0cc..868d1fc9e 100644 --- a/src/core/drive/history.js +++ b/src/core/drive/history.js @@ -1,11 +1,10 @@ -import { nextMicrotask, uuid } from "../../util" +import { uuid } from "../../util" export class History { location restorationIdentifier = uuid() restorationData = {} started = false - pageLoaded = false currentIndex = 0 constructor(delegate) { @@ -15,7 +14,6 @@ export class History { start() { if (!this.started) { addEventListener("popstate", this.onPopState, false) - addEventListener("load", this.onPageLoad, false) this.currentIndex = history.state?.turbo?.restorationIndex || 0 this.started = true this.replace(new URL(window.location.href)) @@ -25,7 +23,6 @@ export class History { stop() { if (this.started) { removeEventListener("popstate", this.onPopState, false) - removeEventListener("load", this.onPageLoad, false) this.started = false } } @@ -81,32 +78,18 @@ export class History { // Event handlers onPopState = (event) => { - if (this.shouldHandlePopState()) { - const { turbo } = event.state || {} - if (turbo) { - this.location = new URL(window.location.href) - const { restorationIdentifier, restorationIndex } = turbo - this.restorationIdentifier = restorationIdentifier - const direction = restorationIndex > this.currentIndex ? "forward" : "back" - this.delegate.historyPoppedToLocationWithRestorationIdentifierAndDirection(this.location, restorationIdentifier, direction) - this.currentIndex = restorationIndex - } + const { turbo } = event.state || {} + this.location = new URL(window.location.href) + + if (turbo) { + const { restorationIdentifier, restorationIndex } = turbo + this.restorationIdentifier = restorationIdentifier + const direction = restorationIndex > this.currentIndex ? "forward" : "back" + this.delegate.historyPoppedToLocationWithRestorationIdentifierAndDirection(this.location, restorationIdentifier, direction) + this.currentIndex = restorationIndex + } else { + this.currentIndex++ + this.delegate.historyPoppedWithEmptyState(this.location) } } - - onPageLoad = async (_event) => { - await nextMicrotask() - this.pageLoaded = true - } - - // Private - - shouldHandlePopState() { - // Safari dispatches a popstate event after window's load event, ignore it - return this.pageIsLoaded() - } - - pageIsLoaded() { - return this.pageLoaded || document.readyState == "complete" - } } diff --git a/src/core/drive/navigator.js b/src/core/drive/navigator.js index 195b46a48..454ce766f 100644 --- a/src/core/drive/navigator.js +++ b/src/core/drive/navigator.js @@ -1,6 +1,6 @@ import { getVisitAction } from "../../util" import { FormSubmission } from "./form_submission" -import { expandURL, getAnchor, getRequestURL } from "../url" +import { expandURL } from "../url" import { Visit } from "./visit" import { PageSnapshot } from "./page_snapshot" @@ -139,20 +139,10 @@ export class Navigator { delete this.currentVisit } + // Same-page links are no longer handled with a Visit. + // This method is still needed for Turbo Native adapters. 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)) - ) - } - - visitScrolledToSamePageLocation(oldURL, newURL) { - this.delegate.visitScrolledToSamePageLocation(oldURL, newURL) + return false } // Visits diff --git a/src/core/drive/visit.js b/src/core/drive/visit.js index 59896b630..092b73b01 100644 --- a/src/core/drive/visit.js +++ b/src/core/drive/visit.js @@ -85,7 +85,6 @@ export class Visit { this.snapshot = snapshot this.snapshotHTML = snapshotHTML this.response = response - this.isSamePage = this.delegate.locationWithActionIsSamePage(this.location, this.action) this.isPageRefresh = this.view.isPageRefresh(this) this.visitCachedSnapshot = visitCachedSnapshot this.willRender = willRender @@ -113,10 +112,6 @@ export class Visit { return this.history.getRestorationDataForIdentifier(this.restorationIdentifier) } - get silent() { - return this.isSamePage - } - start() { if (this.state == VisitState.initialized) { this.recordTimingMetric(TimingMetric.visitStart) @@ -253,7 +248,7 @@ export class Visit { const isPreview = this.shouldIssueRequest() this.render(async () => { this.cacheSnapshot() - if (this.isSamePage || this.isPageRefresh) { + if (this.isPageRefresh) { this.adapter.visitRendered(this) } else { if (this.view.renderPromise) await this.view.renderPromise @@ -281,17 +276,6 @@ export class Visit { } } - goToSamePageAnchor() { - if (this.isSamePage) { - this.render(async () => { - this.cacheSnapshot() - this.performScroll() - this.changeHistory() - this.adapter.visitRendered(this) - }) - } - } - // Fetch request delegate prepareRequest(request) { @@ -353,9 +337,6 @@ export class Visit { } else { this.scrollToAnchor() || this.view.scrollToTop() } - if (this.isSamePage) { - this.delegate.visitScrolledToSamePageLocation(this.view.lastRenderedLocation, this.location) - } this.scrolled = true } @@ -394,9 +375,7 @@ export class Visit { } shouldIssueRequest() { - if (this.isSamePage) { - return false - } else if (this.action == "restore") { + if (this.action == "restore") { return !this.hasCachedSnapshot() } else { return this.willRender diff --git a/src/core/native/browser_adapter.js b/src/core/native/browser_adapter.js index c071f9ac5..fc99ce21e 100644 --- a/src/core/native/browser_adapter.js +++ b/src/core/native/browser_adapter.js @@ -24,7 +24,6 @@ export class BrowserAdapter { visit.loadCachedSnapshot() visit.issueRequest() - visit.goToSamePageAnchor() } visitRequestStarted(visit) { diff --git a/src/core/session.js b/src/core/session.js index a43112831..1e9f1c7bd 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -216,6 +216,12 @@ export class Session { } } + historyPoppedWithEmptyState(location) { + this.history.replace(location) + this.view.lastRenderedLocation = location + this.view.cacheSnapshot() + } + // Scroll observer delegate scrollPositionChanged(position) { @@ -260,7 +266,7 @@ export class Session { // Navigator delegate allowsVisitingLocationWithAction(location, action) { - return this.locationWithActionIsSamePage(location, action) || this.applicationAllowsVisitingLocation(location) + return this.applicationAllowsVisitingLocation(location) } visitProposedToLocation(location, options) { @@ -276,9 +282,7 @@ export class Session { this.view.markVisitDirection(visit.direction) } extendURLWithDeprecatedProperties(visit.location) - if (!visit.silent) { - this.notifyApplicationAfterVisitingLocation(visit.location, visit.action) - } + this.notifyApplicationAfterVisitingLocation(visit.location, visit.action) } visitCompleted(visit) { @@ -287,14 +291,6 @@ export class Session { this.notifyApplicationAfterPageLoad(visit.getTimingMetrics()) } - locationWithActionIsSamePage(location, action) { - return this.navigator.locationWithActionIsSamePage(location, action) - } - - visitScrolledToSamePageLocation(oldURL, newURL) { - this.notifyApplicationAfterVisitingSamePageLocation(oldURL, newURL) - } - // Form submit observer delegate willSubmitForm(form, submitter) { @@ -334,9 +330,7 @@ export class Session { // Page view delegate viewWillCacheSnapshot() { - if (!this.navigator.currentVisit?.silent) { - this.notifyApplicationBeforeCachingSnapshot() - } + this.notifyApplicationBeforeCachingSnapshot() } allowsImmediateRender({ element }, options) { @@ -428,15 +422,6 @@ export class Session { }) } - notifyApplicationAfterVisitingSamePageLocation(oldURL, newURL) { - dispatchEvent( - new HashChangeEvent("hashchange", { - oldURL: oldURL.toString(), - newURL: newURL.toString() - }) - ) - } - notifyApplicationAfterFrameLoad(frame) { return dispatch("turbo:frame-load", { target: frame }) } diff --git a/src/tests/functional/navigation_tests.js b/src/tests/functional/navigation_tests.js index 6648558ef..ef70a1256 100644 --- a/src/tests/functional/navigation_tests.js +++ b/src/tests/functional/navigation_tests.js @@ -299,7 +299,7 @@ test("following a same-origin link inside an SVG element", async ({ page }) => { await page.keyboard.press("Enter") await expect(page).toHaveURL(withPathname("/src/tests/fixtures/one.html")) - expect(await visitAction(page)).toEqual("advance") + expect(await visitAction(page)).toEqual("load") }) test("following a cross-origin link inside an SVG element", async ({ page }) => { diff --git a/src/util.js b/src/util.js index 13ffb6b40..5a9024a1f 100644 --- a/src/util.js +++ b/src/util.js @@ -248,6 +248,7 @@ export function findLinkFromClickTarget(target) { const link = findClosestRecursively(target, "a[href], a[xlink\\:href]") if (!link) return null + if (link.href.startsWith("#")) return null if (link.hasAttribute("download")) return null const linkTarget = link.getAttribute("target")