Location not updated after redirect

[armatys]

Steps to recreate:

• open a home page H which has a link to page A
• click on the link A
• the link returns a 302 redirect to page B
• page B has loaded (it also has turbolinks-location header set)

Result (on Android, using turbo-android):

• window.location is still pointing to page A

Expected (web browser implementation works as expected):

• window.location is pointing to page B

The additional problem: From page B navigate to other pages. Then navigate back to page B. Instead of loading page B directly, turbolinks will load page A and then follow the redirect. The problem is that if you have a form on page B, any input data entered by the user will be gone.

Should there be a call to visit.followRedirect() in turbo_bridge.js in visitCompleted method? This call is present in browser_adapter.ts and when using a web browser, the behavior is as expected (window.location is pointing to page B, and form input data is properly restored).

Tested with:

• Turbo-android version from commit c3503d8
• Turbolinks version 5.2.1

If needed I can provide a demo.

P.s. I've observed this behavior also in the old turbolinks-android

[jayohms]

@armatys Thanks for the detailed bug report, appreciate it 👍 . I need to think about this a bit more on the best solution.

This gets a little tricky because a TurboWebFragment destination location is considered immutable. So we'll probably need to replace the initial TurboWebFragment destination (before the redirect) with a new TurboWebFragment destination (after the redirect).

I'll also need to talk with the iOS team to make sure that we handle redirects in a consistent way.

I'll let you know if I need a demo, but I should be able to replicate this easily. I should have some time to look at this more closely next week.

[jayohms]

An update on this: we're planning to fix this issue in the core Turbo framework, not in the mobile adapters.

When a redirect occurs during a visit, Turbo will propose a new replace visit with the redirected response. This work is planned for an upcoming Turbo beta and may not require any changes in the mobile adapters, but isn't certain yet.

Keeping this open until it's addressed in the core framework.

[nikatlas]

I happen to have the same problem. As a suggestion, I would appreciate more control over the web client. (PS: the 500ms delay in shouldProposeThrottledVisit seems awkward to me, is it related to this?)

[jayohms]

@nikatlas This is scheduled to be fixed in the next Turbo web beta.

As a suggestion, I would appreciate more control over the web client.

Can you elaborate? I'm not sure I understand what you mean.

the 500ms delay in shouldProposeThrottledVisit seems awkward to me, is it related to this?

This is unrelated. The 500ms delay is meant to avoid some quirks we've seen in WebView and prevent double-taps by users, which is undesired.

Currently, the Android (and iOS) mobile frameworks don't ever see the redirected urls - and this is what we intend to fix.

[nikatlas]

@jayohms I see thanks for quick response.

I was thinking that the underlying webClient has the information because navigating the site on chrome i can see all the requests/redirects.

I can see the TurboWebViewClient as an inner class in TurboSession. I suggest you add some flexibility to enable easily extending/overriding the WebViewClient especially in early stages(beta).

Personal debugging

Sharing my dreadful experience before landing here(gonna remove later if irrelevant, but i suppose its some feedback): Here it overrides the shouldOverrideUrlLoading. And i was thinking this is a reason i cannot see the redirects. Throttling based on 500 ms seems strange cause webclient is supposed to do this(not sure but 300ms delay for the long press catch). So i was thinking whether redirections are getting throttled. I cannot debug this code so i didnt know they dont reach here. ```kotlin /** * Turbo will not call adapter.visitProposedToLocation in some cases, * like target=_blank or when the domain doesn't match. We still route those here. * This is only called when links within a webView are clicked and during a * redirect while cold booting. * http://stackoverflow.com/a/6739042/3280911 */ override fun shouldOverrideUrlLoading(view: WebView, request: WebResourceRequest): Boolean { val location = request.url.toString() val isColdBootRedirect = isColdBooting && currentVisit?.location != location val shouldOverride = isReady || isColdBootRedirect // Don't allow onPageFinished to process its // callbacks if a cold boot was blocked. if (isColdBootRedirect) { logEvent("coldBootRedirect", "location" to location) reset() } ->> if (shouldOverride && shouldProposeThrottledVisit()) { // Replace the cold boot destination on a redirect // since the original url isn't visitable. val options = when (isColdBootRedirect) { true -> TurboVisitOptions(action = TurboVisitAction.REPLACE) else -> TurboVisitOptions(action = TurboVisitAction.ADVANCE) } visitProposedToLocation(location, options.toJson()) } logEvent("shouldOverrideUrlLoading", "location" to location, "shouldOverride" to shouldOverride) return shouldOverride <-- gonna be true } ``` ```kotlin /** * Prevents firing twice in a row within a few milliseconds of each other, which * happens sometimes. So we check for a slight delay between requests, which is * plenty of time to allow for a user to click the same link again. */ private fun shouldProposeThrottledVisit(): Boolean { val limit = 500 val currentTime = Date().time return (currentTime - previousOverrideUrlTime > limit).also { previousOverrideUrlTime = currentTime } } ```

[jayohms]

Most url visits don't go through the TurboWebViewClient at all. Once the initial page load is complete, most visits are handled completely through the Turbo javascript and fetch() calls (which is where we need to address the redirect bug).

I doubt we'll ever open up the internal WebViewClient, especially for the intent to let apps address bugs - those should be fixed and addressed in the library. The TurboWebViewClient handles a lot of thorny issues that we've experienced over ~6 years of using Turbolinks and Turbo in production and letting apps change this logic would just make it more difficult to support changes in Turbo and Android WebView going forward.

If there are places where you like to see better debug logging, please let me know.

We're aware of the issue you experienced and it's well documented here and in the linked PR, and we'll get it addressed soon 👍 . Turbo is still considered beta, so some bumps in the road are to be expected.

[jaysson]

Is there an update on this issue?

[jayohms]

This is now fixed using Turbo v7.0.0-rc.3 and the latest Android and iOS library versions. Thanks @jaysson 👍

Fix: https://github.com/hotwired/turbo/pull/328