Skip to content

Commit

Permalink
Merge pull request hotwired#224 from seanpdoyle/frame-src-attribute-t…
Browse files Browse the repository at this point in the history
…ests

Frames: Set `src` when a `<form>` redirects frame
  • Loading branch information
sstephenson committed Apr 8, 2021
2 parents d1c4936 + 2cd1407 commit 2f9a281
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 5 deletions.
17 changes: 14 additions & 3 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
private resolveVisitPromise = () => {}
private connected = false
private hasBeenLoaded = false
private settingSourceURL = false

constructor(element: FrameElement) {
this.element = element
Expand Down Expand Up @@ -70,7 +71,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
}

async loadSourceURL() {
if (this.isActive && this.sourceURL != this.currentURL) {
if (!this.settingSourceURL && this.isActive && this.sourceURL != this.currentURL) {
const previousURL = this.currentURL
this.currentURL = this.sourceURL
if (this.sourceURL) {
Expand All @@ -87,9 +88,13 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
}
}

async loadResponse(response: FetchResponse) {
async loadResponse(fetchResponse: FetchResponse) {
if (fetchResponse.redirected) {
this.sourceURL = fetchResponse.response.url
}

try {
const html = await response.responseHTML
const html = await fetchResponse.responseHTML
if (html) {
const { body } = parseHTMLDocument(html)
const snapshot = new Snapshot(await this.extractForeignFrameElement(body))
Expand Down Expand Up @@ -294,6 +299,12 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
}
}

set sourceURL(sourceURL: string | undefined) {
this.settingSourceURL = true
this.element.src = sourceURL ?? null
this.settingSourceURL = false
}

get loadingStyle() {
return this.element.loading
}
Expand Down
8 changes: 7 additions & 1 deletion src/tests/functional/form_submission_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,20 @@ export class FormSubmissionTests extends TurboDriveTestCase {
}

async "test frame form submission with redirect response"() {
const path = await this.attributeForSelector("#frame form.redirect input[name=path]", "value") || ""
const url = new URL(path, "http://localhost:9000")
url.searchParams.set("enctype", "application/x-www-form-urlencoded;charset=UTF-8")

const button = await this.querySelector("#frame form.redirect input[type=submit]")
await button.click()
await this.nextBeat

const message = await this.querySelector("#frame div.message")
this.assert.notOk(await this.hasSelector("#frame form.redirect"))
this.assert.equal(await message.getVisibleText(), "Frame redirected")
this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html", "does not redirect _top")
this.assert.notOk(await this.search, "does not redirect _top")
this.assert.equal(await this.attributeForSelector("#frame", "src"), url.href, "redirects the target frame")
}

async "test frame form submission with empty created response"() {
Expand Down
15 changes: 14 additions & 1 deletion src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export class FrameTests extends TurboDriveTestCase {

const frame = await this.querySelector("turbo-frame#frame")
this.assert.equal(await frame.getAttribute("data-loaded-from"), currentPath)
this.assert.equal(await frame.getAttribute("src"), await this.propertyForSelector("#hello a", "href"))
}

async "test a frame whose src references itself does not infinitely loop"() {
Expand All @@ -39,7 +40,10 @@ export class FrameTests extends TurboDriveTestCase {
}

async "test following a link within a descendant frame whose ancestor declares a target set navigates the descendant frame"() {
await this.clickSelector("#nested-root[target=frame] #nested-child a:not([data-turbo-frame])")
const link = await this.querySelector("#nested-root[target=frame] #nested-child a:not([data-turbo-frame])")
const href = await link.getProperty("href")

await link.click()
await this.nextBeat

const frame = await this.querySelector("#frame h2")
Expand All @@ -48,6 +52,9 @@ export class FrameTests extends TurboDriveTestCase {
this.assert.equal(await frame.getVisibleText(), "Frames: #frame")
this.assert.equal(await nestedRoot.getVisibleText(), "Frames: #nested-root")
this.assert.equal(await nestedChild.getVisibleText(), "Frame: Loaded")
this.assert.equal(await this.attributeForSelector("#frame", "src"), null)
this.assert.equal(await this.attributeForSelector("#nested-root", "src"), null)
this.assert.equal(await this.attributeForSelector("#nested-child", "src"), href)
}

async "test following a link that declares data-turbo-frame within a frame whose ancestor respects the override"() {
Expand All @@ -56,14 +63,20 @@ export class FrameTests extends TurboDriveTestCase {

const frameText = await this.querySelector("body > h1")
this.assert.equal(await frameText.getVisibleText(), "One")
this.assert.notOk(await this.hasSelector("#frame"))
this.assert.notOk(await this.hasSelector("#nested-root"))
this.assert.notOk(await this.hasSelector("#nested-child"))
}

async "test following a link within a frame with target=_top navigates the page"() {
this.assert.equal(await this.attributeForSelector("#navigate-top" ,"src"), null)

await this.clickSelector("#navigate-top a")
await this.nextBeat

const frameText = await this.querySelector("body > h1")
this.assert.equal(await frameText.getVisibleText(), "One")
this.assert.notOk(await this.hasSelector("#navigate-top"))
}

async "test following a link to a page with a <turbo-frame recurse> which lazily loads a matching frame"() {
Expand Down
16 changes: 16 additions & 0 deletions src/tests/helpers/functional_test_case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,18 @@ export class FunctionalTestCase extends InternTestCase {
return this.evaluate(element => element.innerHTML, element)
}

async attributeForSelector(selector: string, attributeName: string) {
const element = await this.querySelector(selector)

return await element.getAttribute(attributeName)
}

async propertyForSelector(selector: string, attributeName: string) {
const element = await this.querySelector(selector)

return await element.getProperty(attributeName)
}

get scrollPosition(): Promise<{ x: number, y: number }> {
return this.evaluate(() => ({ x: window.scrollX, y: window.scrollY }))
}
Expand Down Expand Up @@ -94,6 +106,10 @@ export class FunctionalTestCase extends InternTestCase {
return this.evaluate(() => location.pathname)
}

get search(): Promise<string> {
return this.evaluate(() => location.search)
}

get searchParams(): Promise<URLSearchParams> {
return this.evaluate(() => location.search).then(search => new URLSearchParams(search))
}
Expand Down

0 comments on commit 2f9a281

Please sign in to comment.