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

Links are not intercepted with target="_blank" on iOS #221

Open
adamkobor opened this issue Sep 10, 2024 · 4 comments
Open

Links are not intercepted with target="_blank" on iOS #221

adamkobor opened this issue Sep 10, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@adamkobor
Copy link
Contributor

The problem: on iOS, if you have a link with target="_blank" inside your webview, it won't open by clicking on it (you have to hold your thumb on the link, then a popup will appear, where you can choose to open it), which is the default behaviour of the webview on iOS, AFAIK.
That wouldn't be necessarily a problem, because it could be at least worked around by intercepting the request, and opening it in a browser, programatically, however those requests are not even intercepted on iOS (they're on Android), because of this condition:

decidePolicyForNavigationAction.targetFrame?.mainFrame == true

I just wanted to ask if there is a reason behind this, or if it could be removed, or at least making it conditional (depending on some property from iOSWebsettings, for example)?
I would be glad to open a PR, my original idea was to introduce something like allowNonMainFrameInterceptions which would be false by default, and if it's true, then the aforementioned condition in that if could be bypassed.
WDYT @KevinnZou ?

@adamkobor adamkobor changed the title Links are not working with target="_blank" on iOS Links are not intercepted with target="_blank" on iOS Sep 10, 2024
@KevinnZou
Copy link
Owner

@adamkobor Thank you for your feedback! The reason I only intercept the mainFrame is that the requestInterceptor is specifically designed to intercept URL changes, not all requests. If you need to intercept other requests as well, you can add a property to the iOSWebsettings. However, you may also need to support this feature for the Android side which requires you to override the shouldInterceptRequest for AccompanistWebViewClient.

@KevinnZou KevinnZou self-assigned this Sep 11, 2024
@KevinnZou KevinnZou added the enhancement New feature or request label Sep 11, 2024
@adamkobor
Copy link
Contributor Author

adamkobor commented Sep 12, 2024

@KevinnZou I'm a little bit confused, because the Android implementation works already differently. For example, it doesn't do the same check in advance:
iOS:

decidePolicyForNavigationAction.targetFrame?.mainFrame == true

Android:
if (isRedirect || request == null || navigator.requestInterceptor == null) {

I think the reason is that the interception works basically in a different way on Android an iOS, and none of them is actually a request interception, rather a navigation interception, XHR/image/etc requests are actually not "caught" on any platform.

Based on my thorough debugging, the library will intercept requests/navigation actions in the following cases:

iOS

Request type w/ mainFrame == true check (actual version) w/ targetFrame?.mainFrame != false check
standard links targeting the same frame & redirects
links with target="_blank" attr ❌ (they're not even working)
iframe src="..."
XHR/Image/Font/etc.

Android

Request type w/ actual version, inside shouldOverrideUrlLoading inside shouldInterceptRequest
standard links targeting the same frame & redirects
links with target="_blank" attr
iframe src="..." ⚠️ not always, seems to be flaky (?)
XHR/Image/Font/etc.

According to this, I would say that implementing the shouldInterceptRequest on Android would not make any sense, because then it would catch every requests, not just the navigation related ones, and it would be completely different from the behaviour on iOS.
However, if the iOS version would check if targetFrame?.mainFrame != false, then it would be identical to the current Android behaviour.

@adamkobor
Copy link
Contributor Author

@KevinnZou I've updated my comment above (and also created a PR), because I found out that removing that check won't be the right solution, however handling null values there is the correct approach IMO

@KevinnZou
Copy link
Owner

@adamkobor Thanks for your contribution! I will review and release it next week!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants