From c458b47c9f4c4bf48ae26f50a3d1bc88ad448039 Mon Sep 17 00:00:00 2001 From: Dom Christie Date: Tue, 26 Sep 2023 08:18:21 +0100 Subject: [PATCH] Let developers access the initiator element in visit/fetch events (#2) * Prevent errors when communicating with iOS webview. To successfully communicate with the iOS view, messages need to be of a type supported by the structuredClone algorithm. The new TransferableVisitOptions type ensures that VisitOptions passed to the adapter conform to this type. The StructuredCloneValue type has been created based on https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm#supported_types * Help clarify VisitOptions values unsupported by postMessage * Let developers access the initiator element in visit/fetch events * Ensure all original VisitOptions are passed to Visit constructor * Update frame test event targets * Fix errors when navigating back/forward. Ensure Navigator#currentVisitOptions is always present * Contain VisitOption munging within Navigator * Lint * Reset currentVisitOptions even if visit proposal fails * Private methods * Raise error if visit options are not transferable to native adapters * Clarify proposed visit options * Lint * Sanitize all values * Ensure proposedVisitOptions is always set * Dispatch visit events on form elements * Fix merge * Unnecessary eslint rule --- src/core/drive/navigator.js | 30 ++++++++++++++++++++++-- src/core/drive/visit.js | 6 +++-- src/core/session.js | 22 ++++++++++------- src/tests/functional/frame_tests.js | 4 ++-- src/tests/functional/navigation_tests.js | 25 ++++++++++++++++++++ 5 files changed, 72 insertions(+), 15 deletions(-) diff --git a/src/core/drive/navigator.js b/src/core/drive/navigator.js index be81794fd..7bdfd5df1 100644 --- a/src/core/drive/navigator.js +++ b/src/core/drive/navigator.js @@ -5,14 +5,18 @@ import { Visit } from "./visit" import { PageSnapshot } from "./page_snapshot" export class Navigator { + proposedVisitOptions = {} + constructor(delegate) { this.delegate = delegate } proposeVisit(location, options = {}) { - if (this.delegate.allowsVisitingLocationWithAction(location, options.action)) { + if (this.delegate.allowsVisitingLocation(location, options)) { if (locationIsVisitable(location, this.view.snapshot.rootLocation)) { - this.delegate.visitProposedToLocation(location, options) + this.#withTransferableVisitOptions(options, (transferableOptions) => { + this.delegate.visitProposedToLocation(location, transferableOptions) + }) } else { window.location.href = location.toString() } @@ -23,6 +27,7 @@ export class Navigator { this.stop() this.currentVisit = new Visit(this, expandURL(locatable), restorationIdentifier, { referrer: this.location, + ...this.proposedVisitOptions, ...options }) this.currentVisit.start() @@ -80,6 +85,7 @@ export class Navigator { const { statusCode, redirected } = fetchResponse const action = this.getActionForFormSubmission(formSubmission) const visitOptions = { + initiator: formSubmission.formElement, action, shouldCacheSnapshot, response: { statusCode, responseHTML, redirected } @@ -154,4 +160,24 @@ export class Navigator { getActionForFormSubmission({ submitter, formElement }) { return getVisitAction(submitter, formElement) || "advance" } + + #withTransferableVisitOptions(options, callback) { + this.proposedVisitOptions = options + try { + callback.call(this, this.#sanitizeVisitOptionsForTransfer(options)) + } finally { + this.proposedVisitOptions = {} + } + } + + #sanitizeVisitOptionsForTransfer(options) { + return Object.entries(options).reduce((sanitized, [key, value]) => { + try { + sanitized[key] = window.structuredClone(value) + } catch (_) { + // Ignore non-transferable values + } + return sanitized + }, {}) + } } diff --git a/src/core/drive/visit.js b/src/core/drive/visit.js index 44cfacdd4..b3e519783 100644 --- a/src/core/drive/visit.js +++ b/src/core/drive/visit.js @@ -72,7 +72,8 @@ export class Visit { updateHistory, shouldCacheSnapshot, acceptsStreamResponse, - direction + direction, + initiator } = { ...defaultOptions, ...options @@ -91,6 +92,7 @@ export class Visit { this.shouldCacheSnapshot = shouldCacheSnapshot this.acceptsStreamResponse = acceptsStreamResponse this.direction = direction || Direction[action] + this.initiator = initiator } get adapter() { @@ -166,7 +168,7 @@ export class Visit { if (this.hasPreloadedResponse()) { this.simulateRequest() } else if (this.shouldIssueRequest() && !this.request) { - this.request = new FetchRequest(this, FetchMethod.get, this.location) + this.request = new FetchRequest(this, FetchMethod.get, this.location, undefined, this.initiator) this.request.perform() } } diff --git a/src/core/session.js b/src/core/session.js index 868804c59..fff40cd7c 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -167,13 +167,16 @@ export class Session { const action = this.getActionForLink(link) const acceptsStreamResponse = link.hasAttribute("data-turbo-stream") - this.visit(location.href, { action, acceptsStreamResponse }) + this.visit(location.href, { action, acceptsStreamResponse, initiator: link }) } // Navigator delegate - allowsVisitingLocationWithAction(location, action) { - return this.locationWithActionIsSamePage(location, action) || this.applicationAllowsVisitingLocation(location) + allowsVisitingLocation(location, options) { + return ( + this.locationWithActionIsSamePage(location, options.action) || + this.applicationAllowsVisitingLocation(location, options) + ) } visitProposedToLocation(location, options) { @@ -189,7 +192,7 @@ export class Session { } extendURLWithDeprecatedProperties(visit.location) if (!visit.silent) { - this.notifyApplicationAfterVisitingLocation(visit.location, visit.action, visit.direction) + this.notifyApplicationAfterVisitingLocation(visit.location, visit.action, visit.direction, visit.initiator) } } @@ -294,8 +297,8 @@ export class Session { return !event.defaultPrevented } - applicationAllowsVisitingLocation(location) { - const event = this.notifyApplicationBeforeVisitingLocation(location) + applicationAllowsVisitingLocation(location, options) { + const event = this.notifyApplicationBeforeVisitingLocation(location, options.initiator) return !event.defaultPrevented } @@ -307,15 +310,16 @@ export class Session { }) } - notifyApplicationBeforeVisitingLocation(location) { + notifyApplicationBeforeVisitingLocation(location, target) { return dispatch("turbo:before-visit", { + target, detail: { url: location.href }, cancelable: true }) } - notifyApplicationAfterVisitingLocation(location, action, direction) { - return dispatch("turbo:visit", { detail: { url: location.href, action, direction } }) + notifyApplicationAfterVisitingLocation(location, action, direction, target) { + return dispatch("turbo:visit", { target, detail: { url: location.href, action, direction } }) } notifyApplicationBeforeCachingSnapshot() { diff --git a/src/tests/functional/frame_tests.js b/src/tests/functional/frame_tests.js index bd06f2d15..c0406a43d 100644 --- a/src/tests/functional/frame_tests.js +++ b/src/tests/functional/frame_tests.js @@ -505,8 +505,8 @@ test("test navigating a frame targeting _top from an outer link fires events", a await page.click("#outside-navigate-top-link") await nextEventOnTarget(page, "outside-navigate-top-link", "turbo:click") - await nextEventOnTarget(page, "html", "turbo:before-fetch-request") - await nextEventOnTarget(page, "html", "turbo:before-fetch-response") + await nextEventOnTarget(page, "outside-navigate-top-link", "turbo:before-fetch-request") + await nextEventOnTarget(page, "outside-navigate-top-link", "turbo:before-fetch-response") await nextEventOnTarget(page, "html", "turbo:before-render") await nextEventOnTarget(page, "html", "turbo:render") await nextEventOnTarget(page, "html", "turbo:load") diff --git a/src/tests/functional/navigation_tests.js b/src/tests/functional/navigation_tests.js index f18a7ea1e..9fad4407a 100644 --- a/src/tests/functional/navigation_tests.js +++ b/src/tests/functional/navigation_tests.js @@ -11,6 +11,7 @@ import { nextBody, nextEventNamed, noNextEventNamed, + nextEventOnTarget, pathname, pathnameForIFrame, readEventLogs, @@ -492,3 +493,27 @@ test("test ignores forms with a [target] attribute that target an iframe with [n assert.equal(pathname(page.url()), "/src/tests/fixtures/one.html") }) + +test("test visit events are dispatched on links", async ({ page }) => { + await page.click("#same-origin-unannotated-link") + await nextEventOnTarget(page, "same-origin-unannotated-link", "turbo:before-visit") + await nextEventOnTarget(page, "same-origin-unannotated-link", "turbo:visit") +}) + +test("test visit events are dispatched on forms", async ({ page }) => { + await page.click("#same-origin-unannotated-form button") + await nextEventOnTarget(page, "same-origin-unannotated-form", "turbo:before-visit") + await nextEventOnTarget(page, "same-origin-unannotated-form", "turbo:visit") +}) + +test("test fetch events are dispatched on links", async ({ page }) => { + await page.click("#same-origin-unannotated-link") + await nextEventOnTarget(page, "same-origin-unannotated-link", "turbo:before-fetch-request") + await nextEventOnTarget(page, "same-origin-unannotated-link", "turbo:before-fetch-response") +}) + +test("test fetch events are dispatched on forms", async ({ page }) => { + await page.click("#same-origin-unannotated-form button") + await nextEventOnTarget(page, "same-origin-unannotated-form", "turbo:before-fetch-request") + await nextEventOnTarget(page, "same-origin-unannotated-form", "turbo:before-fetch-response") +})