Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Form submit with data-turbo="false" #73

Open
tim-vandecasteele opened this issue Jan 22, 2025 · 10 comments
Open

Form submit with data-turbo="false" #73

tim-vandecasteele opened this issue Jan 22, 2025 · 10 comments

Comments

@tim-vandecasteele
Copy link

Don't know if this belongs here, so sorry if this is the wrong place.

I'm wondering what the expected behaviour is of a form with turbo disabled:

<form action="/resources" method="post" data-turbo="false">
  <input type="text" name="institution_id" value="123">
  <button type="submit">submit</button>
</form>

In a regular browser, this submits the form as you would expect, but in my native application, nothing seems to happen. I tried this in the hotwire-native-ios app, and nothing is happening either. As soon as I remove data-turbo="false", the form is successfully submitted.

Is this expected behaviour or a bug?

(the reason I need to disable turbo is because this is a post that redirects to an external url)

@gvarela
Copy link

gvarela commented Jan 22, 2025

We are actually working through the same exact issue. Have yet to figure out the cause. We do not have this same issue in Android.

@jayohms
Copy link
Contributor

jayohms commented Jan 22, 2025

Can you please provide debug logs when attempting to submit the form. Is there any activity in the library?

(the reason I need to disable turbo is because this is a post that redirects to an external url)

This is a design choice that could be changed to multiple options to work around this:

  • Redirect to an internal url that then redirects to the external url. This was recently supported in: Detect cross-origin redirects during visits #70
  • Redirect to an internal url with a (for example) redirect_to query parameter with the external url. The native app can inspect visit proposals for the presence of the query parameter and handle it manually.

@gvarela
Copy link

gvarela commented Jan 22, 2025

For our use case it's an internal redirect and we have an old app that we have slowly been moving more and more over to Turbo. We get absolutely nothing in the logs, nothing in Turbo native is being triggered. I've been trying to figure out where we can hack in some console logging to figure out what might be happening. My current hypothesis is that something is intercepting the form submission and killing the event bubble. I've tried setting break points in the Safari web inspector but, am not seeing anything obvious.

And again we don't see this behavior on Android (say this to hopefully help with where we could look?)

@tim-vandecasteele
Copy link
Author

  • Redirect to an internal url that then redirects to the external url. This was recently supported in: Detect cross-origin redirects during visits #70
  • Redirect to an internal url with a (for example) redirect_to query parameter with the external url. The native app can inspect visit proposals for the presence of the query parameter and handle it manually.

Thanks, super helpful!

I actually didn't see any activity in the logs. It seemed like nothing was happening at all.

@jayohms
Copy link
Contributor

jayohms commented Jan 22, 2025

If it works on Android, then it's likely a result of the default WKNavigationActionPolicy in the library being too restrictive:
https://github.com/hotwired/hotwire-native-ios/blob/main/Source/Turbo/Session/Session.swift#L479

Can you please try to point your app to the custom-web-decision-policy branch in this repo to see if it makes a difference:
main...custom-web-decision-policy

You'll need to implement a HotwireNative.NavigatorDelegate and provide a custom implementation for:

public func webNavigationDecision(for navigationAction: WKNavigationAction) -> WebNavigationDecision {
    if navigationAction.navigationType == .linkActivated {
        return .init(
            policy: .cancel,
            externallyOpenableURL: navigationAction.request.url,
            shouldReloadPage: false
        )
    }

    // All other actions (form submission, iframes, etc) are allowed.
    return .init(
        policy: .allow,
        externallyOpenableURL: nil,
        shouldReloadPage: false
    )
}

This is a branch we're experimenting to allow more flexibility with how WKWebView handles actions that Turbo doesn't handle by default.

Here's where the NavigatorDelegate is provided in the demo app:

extension SceneController: NavigatorDelegate {

cc @svara

@gvarela
Copy link

gvarela commented Jan 22, 2025

Yes this appears to fix it! Is this a feature you are currently working on implementing?

@jayohms
Copy link
Contributor

jayohms commented Jan 22, 2025

We recently added this branch to support some legacy features in our Basecamp iOS app. Our plan is to provide a comprehensive solution for this and allow navigation routing decisions to be more flexible, like the Android library. We'll likely begin this work in the next month or so. I can't guarantee how much the implementation will/won't change, but you're free to use the branch in the meantime. It won't be deleted until we have an alternative solution.

@gvarela
Copy link

gvarela commented Jan 22, 2025

Potentially related but, was going to create a separate issue. So, while I have you.

This PR #70 broke the behavior of an iFrame we have on one of our pages. We have some third-party functionality embedded via an iframe when we initialize it it appears to be triggering this new cross-origin detection and tries to open the iFrame in Safari. Would the proposed change allow us to prevent this from happening on non-main frames?

@jayohms
Copy link
Contributor

jayohms commented Jan 22, 2025

To be honest, I'm not sure. It's surprising to hear that an iframe is triggering a Turbo visit request that fails. The cross-origin redirect detection should only happen after a Turbo visit fails due to CORS.

My first guess is an an application bug that creates a Turbo visit associated with the iframe.

If you can provide a reproducible example in the demo site, we can take a look: https://github.com/hotwired/hotwire-native-demo

@gvarela
Copy link

gvarela commented Jan 23, 2025

@jayohms here is an example hotwired/hotwire-native-demo#13 it only happens during a ColdBootVisit

Adding

        guard let targetFrame = navigationAction.targetFrame, targetFrame.isMainFrame else {
            decisionHandler(.cancel)
            return
        }

into ColdBootVisit func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void)

Fixes the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants