Skip to content

Commit

Permalink
Fix double before-fetch-request dispatch during reload (#740)
Browse files Browse the repository at this point in the history
Closes #739

Issue #739 mentions the issue at the root of the unexpected behavior:

> The reason is that `reload()` calls `this.removeAttribute("complete")`
> and the frame watches attribute changes and calls
> `this.loadSourceURL()` when the “complete” attribute is removed.

This commit resolves the issue in several steps.

First, add the `sourceURLReloaded()` callback to the
`FrameElementDelegate` interface.

Next, move the `FrameElement.reload()` implementation from
`FrameElement.reload()` to the `FrameController.sourceURLReloaded()`.

Finally, wrap the implementation's call to
`this.element.removeAttribute("complete")` within a
`ignoringChangesToAttribute("complete", () => ...)` block so that the
change to the attribute is temporarily ignored.
  • Loading branch information
seanpdoyle authored Sep 27, 2022
1 parent 399658a commit 732db00
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 5 deletions.
10 changes: 10 additions & 0 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@ export class FrameController
}
}

sourceURLReloaded() {
const { src } = this.element
this.ignoringChangesToAttribute("complete", () => {
this.element.removeAttribute("complete")
})
this.element.src = null
this.element.src = src
return this.element.loaded
}

completeChanged() {
if (this.isIgnoringChangesTo("complete")) return

Expand Down
7 changes: 2 additions & 5 deletions src/elements/frame_element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface FrameElementDelegate extends LinkClickObserverDelegate, FormSub
completeChanged(): void
loadingStyleChanged(): void
sourceURLChanged(): void
sourceURLReloaded(): Promise<void>
disabledChanged(): void
loadResponse(response: FetchResponse): void
fetchResponseLoaded: (fetchResponse: FetchResponse) => void
Expand Down Expand Up @@ -63,11 +64,7 @@ export class FrameElement extends HTMLElement {
}

reload(): Promise<void> {
const { src } = this
this.removeAttribute("complete")
this.src = null
this.src = src
return this.loaded
return this.delegate.sourceURLReloaded()
}

attributeChangedCallback(name: string) {
Expand Down
19 changes: 19 additions & 0 deletions src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,25 @@ test("test navigating a frame targeting _top from an outer link fires events", a
assert.equal(otherEvents.length, 0, "no more events")
})

test("test invoking .reload() re-fetches the frame's content", async ({ page }) => {
await page.click("#link-frame")
await nextEventOnTarget(page, "frame", "turbo:frame-load")
await page.evaluate(() => (document.getElementById("frame") as any).reload())

const dispatchedEvents = await readEventLogs(page)

assert.deepEqual(
dispatchedEvents.map(([name, _, id]) => [id, name]),
[
["frame", "turbo:before-fetch-request"],
["frame", "turbo:before-fetch-response"],
["frame", "turbo:before-frame-render"],
["frame", "turbo:frame-render"],
["frame", "turbo:frame-load"],
]
)
})

test("test following inner link reloads frame on every click", async ({ page }) => {
await page.click("#hello a")
await nextEventNamed(page, "turbo:before-fetch-request")
Expand Down

0 comments on commit 732db00

Please sign in to comment.