Skip to content

Commit

Permalink
Disable InstantClick for touch events (#1167)
Browse files Browse the repository at this point in the history
* Check if prefetch request is still valid on mouseleave and touchend events

If it's not and the prefetch delay is not over, we cancel the prefetch
request.

* Disable InstantClick on touch devices

After testing this on iPhone, it seems it can lead to duplicated requests
on touch devices. The culprit seems a `mouseenter` that Safari fires
after a `touchend` event for compatibility reasons.

According to ChatGPT:

> Some browsers may synthesize mouse events (including mouseenter) after
> touch events to ensure compatibility with web content not designed for
> touch interfaces. This means that a mouseenter event might be fired on
> a touch device, usually after a touchend event, as part of the sequence
> to simulate mouse interaction. This behavior can vary between browsers
> and might not always be consistent.

Co-Authored-By: Sean Doyle <[email protected]>

* Remove obsolete tests

* Rename method

---------

Co-authored-by: Sean Doyle <[email protected]>
  • Loading branch information
Alberto Fernández-Capel and seanpdoyle authored Feb 6, 2024
1 parent 3c3eeb8 commit 52c8533
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 33 deletions.
26 changes: 18 additions & 8 deletions src/observers/link_prefetch_observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ import { prefetchCache, cacheTtl } from "../core/drive/prefetch_cache"

export class LinkPrefetchObserver {
started = false
hoverTriggerEvent = "mouseenter"
touchTriggerEvent = "touchstart"
#prefetchedLink = null

constructor(delegate, eventTarget) {
this.delegate = delegate
Expand All @@ -33,27 +32,29 @@ export class LinkPrefetchObserver {
stop() {
if (!this.started) return

this.eventTarget.removeEventListener(this.hoverTriggerEvent, this.#tryToPrefetchRequest, {
this.eventTarget.removeEventListener("mouseenter", this.#tryToPrefetchRequest, {
capture: true,
passive: true
})
this.eventTarget.removeEventListener(this.touchTriggerEvent, this.#tryToPrefetchRequest, {
this.eventTarget.removeEventListener("mouseleave", this.#cancelRequestIfObsolete, {
capture: true,
passive: true
})

this.eventTarget.removeEventListener("turbo:before-fetch-request", this.#tryToUsePrefetchedRequest, true)
this.started = false
}

#enable = () => {
this.eventTarget.addEventListener(this.hoverTriggerEvent, this.#tryToPrefetchRequest, {
this.eventTarget.addEventListener("mouseenter", this.#tryToPrefetchRequest, {
capture: true,
passive: true
})
this.eventTarget.addEventListener(this.touchTriggerEvent, this.#tryToPrefetchRequest, {
this.eventTarget.addEventListener("mouseleave", this.#cancelRequestIfObsolete, {
capture: true,
passive: true
})

this.eventTarget.addEventListener("turbo:before-fetch-request", this.#tryToUsePrefetchedRequest, true)
this.started = true
}
Expand All @@ -69,6 +70,8 @@ export class LinkPrefetchObserver {
const location = getLocationForLink(link)

if (this.delegate.canPrefetchRequestToLocation(link, location)) {
this.#prefetchedLink = link

const fetchRequest = new FetchRequest(
this,
FetchMethod.get,
Expand All @@ -78,12 +81,19 @@ export class LinkPrefetchObserver {
)

prefetchCache.setLater(location.toString(), fetchRequest, this.#cacheTtl)

link.addEventListener("mouseleave", () => prefetchCache.clear(), { once: true })
}
}
}

#cancelRequestIfObsolete = (event) => {
if (event.target === this.#prefetchedLink) this.#cancelPrefetchRequest()
}

#cancelPrefetchRequest = () => {
prefetchCache.clear()
this.#prefetchedLink = null
}

#tryToUsePrefetchedRequest = (event) => {
if (event.target.tagName !== "FORM" && event.detail.fetchOptions.method === "get") {
const cached = prefetchCache.get(event.detail.url.toString())
Expand Down
25 changes: 0 additions & 25 deletions src/tests/functional/link_prefetch_observer_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,6 @@ test("it resets the cache when a link is hovered", async ({ page }) => {
assert.equal(requestCount, 2)
})

test("it prefetches page on touchstart", async ({ page }) => {
await goTo({ page, path: "/hover_to_prefetch.html" })
await assertPrefetchedOnTouchstart({ page, selector: "#anchor_for_prefetch" })
})

test("it does not make a network request when clicking on a link that has been prefetched", async ({ page }) => {
await goTo({ page, path: "/hover_to_prefetch.html" })
await hoverSelector({ page, selector: "#anchor_for_prefetch" })
Expand All @@ -264,26 +259,6 @@ test("it follows the link using the cached response when clicking on a link that
assert.equal(await page.title(), "Prefetched Page")
})

const assertPrefetchedOnTouchstart = async ({ page, selector, callback }) => {
let requestMade = false

page.on("request", (request) => {
callback && callback(request)
requestMade = true
})

const selectorXY = await page.$eval(selector, (el) => {
const { x, y } = el.getBoundingClientRect()
return { x, y }
})

await page.touchscreen.tap(selectorXY.x, selectorXY.y)

await sleep(100)

assertRequestMade(requestMade)
}

const assertPrefetchedOnHover = async ({ page, selector, callback }) => {
let requestMade = false

Expand Down

0 comments on commit 52c8533

Please sign in to comment.