Allow a frame to navigate to previous url

[t27duck]

This is my attempt at a fix based on discussion and the previous PR as well as adding a test for it.

The gist: A frame will only update/make a request if currentURL and sourceURL are different. currentURL was not get updated properly so it was still pointing to the previous value. If sourceURL changes back to the original URL of the frame, the values would match and result in the frame not navigating.

Example:

• Frame gets src set to "a". sourceURL and currentURL are set to "a"
• The frame's src is changed to "b". While sourceURL is updated, currentURL is still "a". Frame navigates to "b".
• The frame's src is changed back to "a". Because currentURL was never changed to "b", navigation doesn't happen because sourceURL == currentURL.

Fixes #245
Fixes #249 (I presume)
Fixes #265 (Reported)

[tleish]

I might think you would want this use the following change instead to ensure that this.element.src and this.currentURL match exactly.

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

[t27duck]

I might think you would want this use the following change instead to ensure that this.element.src and this.currentURL match exactly.

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

Good idea. Updated. (Not like this is ever going to be merged)

Thanks for pointing that out. 👍

[ghiculescu]

Thanks for this, it looks like it fixes #265 too.

[acetinick]

For those needing this fix until its merged, I have published it as separate npm package here if anyone needs it.

https://www.npmjs.com/package/@peopleforce/turbo

[bobmaerten]

I successfully tested on our stack with @acetinick 's module, the revisit problem seems to be gone.
Well played @t27duck!
Hope to have a release soon enough 🙏🏻

[weaverryan]

I patched this PR into my local code and I can also confirm that it fixed the bug ❤️

[sukei]

I patched this PR into my local code and I can also confirm that it fixed the bug ❤️

Hope it get merged soon!

[t27duck]

Hope it get merged soon!

To be perfectly honest, at this point probably DHH is the only one left with merge and release powers for this repo. So the odds of this (or any future releases of turbo) are pretty slim at this point.

[sukei]

Thank you for the status report. I was thinking of waiting, but finally I'll fork it out while waiting for the official support.

[acetinick]

@dhh would you be able to review and merge this PR? It's quite a critical bug that is affecting lots of people. Thanks

[t27duck]

Updated the PR description to better outline what the bug is and why this change is needed.

[Bramjetten]

@t27duck Thanks for fixing this 🙏

@dhh bumped the turbo-rails gem to 0.5.10 two days ago, causing more people to run into this bug now. Is @dhh the only one that can review this PR?

[dhh]

Thanks for working on this @t27duck. I'll get this merged and cut a new release shortly.

[dhh]

Fix included in beta 7 that was just released 👍