Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore UJS <a> clicks and <form> submissions #744

Merged
merged 4 commits into from
Oct 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 13 additions & 23 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import { ViewDelegate, ViewRenderOptions } from "../view"
import { Locatable, getAction, expandURL, urlsAreEqual, locationIsVisitable } from "../url"
import { FormSubmitObserver, FormSubmitObserverDelegate } from "../../observers/form_submit_observer"
import { FrameView } from "./frame_view"
import { LinkClickObserver, LinkClickObserverDelegate } from "../../observers/link_click_observer"
import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
import { FormLinkClickObserver, FormLinkClickObserverDelegate } from "../../observers/form_link_click_observer"
import { FrameRenderer } from "./frame_renderer"
import { TurboClickEvent, session } from "../index"
import { session } from "../index"
import { isAction, Action } from "../types"
import { VisitOptions } from "../drive/visit"
import { TurboBeforeFrameRenderEvent } from "../session"
Expand All @@ -43,14 +43,14 @@ export class FrameController
FormSubmissionDelegate,
FrameElementDelegate,
FormLinkClickObserverDelegate,
LinkClickObserverDelegate,
LinkInterceptorDelegate,
ViewDelegate<FrameElement, Snapshot<FrameElement>>
{
readonly element: FrameElement
readonly view: FrameView
readonly appearanceObserver: AppearanceObserver
readonly formLinkClickObserver: FormLinkClickObserver
readonly linkClickObserver: LinkClickObserver
readonly linkInterceptor: LinkInterceptor
readonly formSubmitObserver: FormSubmitObserver
formSubmission?: FormSubmission
fetchResponseLoaded = (_fetchResponse: FetchResponse) => {}
Expand All @@ -70,7 +70,7 @@ export class FrameController
this.view = new FrameView(this, this.element)
this.appearanceObserver = new AppearanceObserver(this, this.element)
this.formLinkClickObserver = new FormLinkClickObserver(this, this.element)
this.linkClickObserver = new LinkClickObserver(this, this.element)
this.linkInterceptor = new LinkInterceptor(this, this.element)
this.restorationIdentifier = uuid()
this.formSubmitObserver = new FormSubmitObserver(this, this.element)
}
Expand All @@ -84,7 +84,7 @@ export class FrameController
this.loadSourceURL()
}
this.formLinkClickObserver.start()
this.linkClickObserver.start()
this.linkInterceptor.start()
this.formSubmitObserver.start()
}
}
Expand All @@ -94,7 +94,7 @@ export class FrameController
this.connected = false
this.appearanceObserver.stop()
this.formLinkClickObserver.stop()
this.linkClickObserver.stop()
this.linkInterceptor.stop()
this.formSubmitObserver.stop()
}
}
Expand Down Expand Up @@ -204,22 +204,22 @@ export class FrameController
// Form link click observer delegate

willSubmitFormLinkToLocation(link: Element): boolean {
return link.closest("turbo-frame") == this.element && this.shouldInterceptNavigation(link)
return this.shouldInterceptNavigation(link)
}

submittedFormLinkToLocation(link: Element, _location: URL, form: HTMLFormElement): void {
const frame = this.findFrameElement(link)
if (frame) form.setAttribute("data-turbo-frame", frame.id)
}

// Link click observer delegate
// Link interceptor delegate

willFollowLinkToLocation(element: Element, location: URL, event: MouseEvent) {
return this.shouldInterceptNavigation(element) && this.frameAllowsVisitingLocation(element, location, event)
shouldInterceptLinkClick(element: Element, _location: string, _event: MouseEvent) {
return this.shouldInterceptNavigation(element)
}

followedLinkToLocation(element: Element, location: URL) {
this.navigateFrame(element, location.href)
linkClickIntercepted(element: Element, location: string) {
this.navigateFrame(element, location)
}

// Form submit observer delegate
Expand Down Expand Up @@ -555,16 +555,6 @@ export class FrameController
return expandURL(root)
}

private frameAllowsVisitingLocation(target: Element, { href: url }: URL, originalEvent: MouseEvent): boolean {
const event = dispatch<TurboClickEvent>("turbo:click", {
target,
detail: { url, originalEvent },
cancelable: true,
})

return !event.defaultPrevented
}

private isIgnoringChangesTo(attributeName: FrameElementObservedAttribute): boolean {
return this.ignoredAttributes.has(attributeName)
}
Expand Down
34 changes: 11 additions & 23 deletions src/core/frames/frame_redirector.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,39 @@
import { FormSubmitObserver, FormSubmitObserverDelegate } from "../../observers/form_submit_observer"
import { FrameElement } from "../../elements/frame_element"
import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
import { expandURL, getAction, locationIsVisitable } from "../url"
import { LinkClickObserver, LinkClickObserverDelegate } from "../../observers/link_click_observer"
import { Session, TurboClickEvent } from "../session"
import { dispatch } from "../../util"

export class FrameRedirector implements LinkClickObserverDelegate, FormSubmitObserverDelegate {
import { Session } from "../session"
export class FrameRedirector implements LinkInterceptorDelegate, FormSubmitObserverDelegate {
readonly session: Session
readonly element: Element
readonly linkClickObserver: LinkClickObserver
readonly linkInterceptor: LinkInterceptor
readonly formSubmitObserver: FormSubmitObserver

constructor(session: Session, element: Element) {
this.session = session
this.element = element
this.linkClickObserver = new LinkClickObserver(this, element)
this.linkInterceptor = new LinkInterceptor(this, element)
this.formSubmitObserver = new FormSubmitObserver(this, element)
}

start() {
this.linkClickObserver.start()
this.linkInterceptor.start()
this.formSubmitObserver.start()
}

stop() {
this.linkClickObserver.stop()
this.linkInterceptor.stop()
this.formSubmitObserver.stop()
}

willFollowLinkToLocation(element: Element, location: URL, event: MouseEvent) {
return this.shouldRedirect(element) && this.frameAllowsVisitingLocation(element, location, event)
shouldInterceptLinkClick(element: Element, _location: string, _event: MouseEvent) {
return this.shouldRedirect(element)
}

followedLinkToLocation(element: Element, url: URL) {
linkClickIntercepted(element: Element, url: string, event: MouseEvent) {
const frame = this.findFrameElement(element)
if (frame) {
frame.delegate.followedLinkToLocation(element, url)
frame.delegate.linkClickIntercepted(element, url, event)
}
}

Expand All @@ -54,16 +52,6 @@ export class FrameRedirector implements LinkClickObserverDelegate, FormSubmitObs
}
}

private frameAllowsVisitingLocation(target: Element, { href: url }: URL, originalEvent: MouseEvent): boolean {
const event = dispatch<TurboClickEvent>("turbo:click", {
target,
detail: { url, originalEvent },
cancelable: true,
})

return !event.defaultPrevented
}

private shouldSubmit(form: HTMLFormElement, submitter?: HTMLElement) {
const action = getAction(form, submitter)
const meta = this.element.ownerDocument.querySelector<HTMLMetaElement>(`meta[name="turbo-root"]`)
Expand Down
57 changes: 57 additions & 0 deletions src/core/frames/link_interceptor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { TurboClickEvent, TurboBeforeVisitEvent } from "../session"

export interface LinkInterceptorDelegate {
shouldInterceptLinkClick(element: Element, url: string, originalEvent: MouseEvent): boolean
linkClickIntercepted(element: Element, url: string, originalEvent: MouseEvent): void
}

export class LinkInterceptor {
readonly delegate: LinkInterceptorDelegate
readonly element: Element
private clickEvent?: Event

constructor(delegate: LinkInterceptorDelegate, element: Element) {
this.delegate = delegate
this.element = element
}

start() {
this.element.addEventListener("click", this.clickBubbled)
document.addEventListener("turbo:click", this.linkClicked)
document.addEventListener("turbo:before-visit", this.willVisit)
}

stop() {
this.element.removeEventListener("click", this.clickBubbled)
document.removeEventListener("turbo:click", this.linkClicked)
document.removeEventListener("turbo:before-visit", this.willVisit)
}

clickBubbled = (event: Event) => {
if (this.respondsToEventTarget(event.target)) {
this.clickEvent = event
} else {
delete this.clickEvent
}
}

linkClicked = <EventListener>((event: TurboClickEvent) => {
if (this.clickEvent && this.respondsToEventTarget(event.target) && event.target instanceof Element) {
if (this.delegate.shouldInterceptLinkClick(event.target, event.detail.url, event.detail.originalEvent)) {
this.clickEvent.preventDefault()
event.preventDefault()
this.delegate.linkClickIntercepted(event.target, event.detail.url, event.detail.originalEvent)
}
}
delete this.clickEvent
})

willVisit = <EventListener>((_event: TurboBeforeVisitEvent) => {
delete this.clickEvent
})

respondsToEventTarget(target: EventTarget | null) {
const element = target instanceof Element ? target : target instanceof Node ? target.parentElement : null
return element && element.closest("turbo-frame, html") == this.element
}
}
4 changes: 2 additions & 2 deletions src/elements/frame_element.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FetchResponse } from "../http/fetch_response"
import { Snapshot } from "../core/snapshot"
import { LinkClickObserverDelegate } from "../observers/link_click_observer"
import { LinkInterceptorDelegate } from "../core/frames/link_interceptor"
import { FormSubmitObserverDelegate } from "../observers/form_submit_observer"

export enum FrameLoadingStyle {
Expand All @@ -10,7 +10,7 @@ export enum FrameLoadingStyle {

export type FrameElementObservedAttribute = keyof FrameElement & ("disabled" | "complete" | "loading" | "src")

export interface FrameElementDelegate extends LinkClickObserverDelegate, FormSubmitObserverDelegate {
export interface FrameElementDelegate extends LinkInterceptorDelegate, FormSubmitObserverDelegate {
connect(): void
disconnect(): void
completeChanged(): void
Expand Down
8 changes: 4 additions & 4 deletions src/observers/form_link_click_observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@ export type FormLinkClickObserverDelegate = {
}

export class FormLinkClickObserver implements LinkClickObserverDelegate {
readonly linkClickObserver: LinkClickObserver
readonly linkInterceptor: LinkClickObserver
readonly delegate: FormLinkClickObserverDelegate

constructor(delegate: FormLinkClickObserverDelegate, element: HTMLElement) {
this.delegate = delegate
this.linkClickObserver = new LinkClickObserver(this, element)
this.linkInterceptor = new LinkClickObserver(this, element)
}

start() {
this.linkClickObserver.start()
this.linkInterceptor.start()
}

stop() {
this.linkClickObserver.stop()
this.linkInterceptor.stop()
}

willFollowLinkToLocation(link: Element, location: URL, originalEvent: MouseEvent): boolean {
Expand Down
1 change: 1 addition & 0 deletions src/observers/form_submit_observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export class FormSubmitObserver {
this.delegate.willSubmitForm(form, submitter)
) {
event.preventDefault()
event.stopImmediatePropagation()
this.delegate.formSubmitted(form, submitter)
}
}
Expand Down
24 changes: 24 additions & 0 deletions src/tests/fixtures/ujs.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>Frame</title>
<script src="/src/tests/fixtures/test.js"></script>
<script type="module">
import Rails from "https://ga.jspm.io/npm:@rails/ujs@7.0.1/lib/assets/compiled/rails-ujs.js"

Rails.start()
</script>
<script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
</head>
<body>
<turbo-frame id="frame">
<h2>Frames: #frame</h2>

<a data-remote="true" href="/src/tests/fixtures/frames/frame.html">navigate #frame to /src/tests/fixtures/frames/frame.html</a>
<form data-remote="true" action="/src/tests/fixtures/frames/frame.html">
<button>navigate #frame to /src/tests/fixtures/frames/frame.html</button>
</form>
</turbo-frame>
</body>
</html>
12 changes: 4 additions & 8 deletions src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { Page, test } from "@playwright/test"
import { assert, Assertion } from "chai"
import {
attributeForSelector,
cancelNextEvent,
hasSelector,
innerHTMLForSelector,
listenForEventOnTarget,
nextAttributeMutationNamed,
noNextAttributeMutationNamed,
nextBeat,
Expand Down Expand Up @@ -442,6 +442,7 @@ test("test navigating a frame from an outer form fires events", async ({ page })
})

test("test navigating a frame from an outer link fires events", async ({ page }) => {
await listenForEventOnTarget(page, "outside-frame-form", "turbo:click")
await page.click("#outside-frame-form")

await nextEventOnTarget(page, "outside-frame-form", "turbo:click")
Expand All @@ -456,14 +457,8 @@ test("test navigating a frame from an outer link fires events", async ({ page })
assert.equal(otherEvents.length, 0, "no more events")
})

test("test canceling a turbo:cilck event falls back to built-in browser navigation", async ({ page }) => {
await cancelNextEvent(page, "turbo:click")
await Promise.all([page.waitForNavigation(), page.click("#link-frame")])

assert.equal(pathname(page.url()), "/src/tests/fixtures/frames/frame.html")
})

test("test navigating a frame from an inner link fires events", async ({ page }) => {
await listenForEventOnTarget(page, "link-frame", "turbo:click")
await page.click("#link-frame")

await nextEventOnTarget(page, "link-frame", "turbo:click")
Expand All @@ -479,6 +474,7 @@ test("test navigating a frame from an inner link fires events", async ({ page })
})

test("test navigating a frame targeting _top from an outer link fires events", async ({ page }) => {
await listenForEventOnTarget(page, "outside-navigate-top-link", "turbo:click")
await page.click("#outside-navigate-top-link")

await nextEventOnTarget(page, "outside-navigate-top-link", "turbo:click")
Expand Down
37 changes: 37 additions & 0 deletions src/tests/functional/ujs_tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { Page, test } from "@playwright/test"
import { assert } from "chai"
import { noNextEventOnTarget } from "../helpers/page"

test.beforeEach(async ({ page }) => {
await page.goto("/src/tests/fixtures/ujs.html")
})

test("test clicking a [data-remote=true] anchor within a turbo-frame", async ({ page }) => {
await assertRequestLimit(page, 1, async () => {
assert.equal(await page.textContent("#frame h2"), "Frames: #frame")

await page.click("#frame a[data-remote=true]")
await noNextEventOnTarget(page, "frame", "turbo:frame-load")

assert.equal(await page.textContent("#frame h2"), "Frames: #frame", "does not navigate the target frame")
})
})

test("test submitting a [data-remote=true] form within a turbo-frame", async ({ page }) => {
await assertRequestLimit(page, 1, async () => {
assert.equal(await page.textContent("#frame h2"), "Frames: #frame")

await page.click("#frame form[data-remote=true] button")
await noNextEventOnTarget(page, "frame", "turbo:frame-load")

assert.equal(await page.textContent("#frame h2"), "Frame: Loaded", "navigates the target frame")
})
})

async function assertRequestLimit(page: Page, count: number, callback: () => Promise<void>) {
let requestsStarted = 0
await page.on("request", () => requestsStarted++)
await callback()

assert.equal(requestsStarted, count, `only submits ${count} requests`)
}
Loading