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

fix(ios): make system open tel, sms, mailto and geo links #881

Merged
merged 1 commit into from
Jun 11, 2021

Conversation

jcesarmobile
Copy link
Member

Platforms affected

ios

Motivation and Context

closes #830

Description

UIWebView handled tel, sms, mailto and geo links, but WKWebView doesn't and if the site you are loading has any of those urls on links it will just try to load them and fail
android also has specific code for handling those app schemes

Testing

app with this code cordova.InAppBrowser.open('https://www.parsippanysbestpizza.com/', '_blank');
has a mail link on top and telephone link on the bottom, without the fix they do nothing, with the fix mail link opens mail app and tel link prompts for making a call

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

@jcesarmobile
Copy link
Member Author

That's unrelated to this PR, please, don't start off-topic conversations.

If you want help, first thing you should do is make it easy to reproduce your issue, it's unclear what your problem is.
Provide a sample app where it can easily reproduced. And if you say it was fixed in cordova-android it would also be helpful to link the PR where it was fixed.
But please, do it in your issue, not in my PR.

Copy link
Member

@NiklasMerz NiklasMerz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LTGM

@dpa99c
Copy link
Contributor

dpa99c commented Jun 11, 2021

@mosabab Your comment was completely off-topic and @jcesarmobile was perfectly polite in his response.
Your response however is rude and unacceptable - if this was my repo, I may well have banned you for such rudeness.
You should remember that future employers scan your github comment history to gain an insight into your character and this does not look good for you.

@jcesarmobile
Copy link
Member Author

@mosabab, this pull request is about tel, sms, mailto and geo schemes not working on iOS when using the WKWebView.
In the description of the pull request I've mentioned that android already has code for handling tel, sms, mailto and geo schemes and I verified that they work fine in Android. Also verified that they used to work on UIWebView (plugin version 3 and older), so yeah, I don't think this pull request and your issue are related because it's a fix for iOS of something that already works on Android and that used to work in older plugin versions of iOS, I hope you now see how they are different too.

I also took the time to look into your issue, but it's not clear what your problem is by reading the description, so that's why I gave you the advice of improving it by providing a sample app. It's an advice, not telling you to do it, but you shouldn't demand a fix if you are not willing to collaborate by making it easy to reproduce. (It's also an advice, of course you can demand whatever you want)

Thanks for the reviews!

@jcesarmobile jcesarmobile merged commit 2fef304 into apache:master Jun 11, 2021
@jcesarmobile jcesarmobile deleted the allow-schemes branch June 11, 2021 12:25
sirineKr pushed a commit to jalios/cordova-plugin-inappbrowser that referenced this pull request Jan 21, 2022
sirineKr pushed a commit to jalios/cordova-plugin-inappbrowser that referenced this pull request Jan 28, 2022
sirineKr pushed a commit to jalios/cordova-plugin-inappbrowser that referenced this pull request Jan 28, 2022
MaximBelov pushed a commit to MaximBelov/cordova-plugin-inappbrowser that referenced this pull request Aug 25, 2022
@ShahinBelim
Copy link

@jcesarmobile, are these changes published? I downloaded the latest version of cordova-plugin-inappbrowser which is 5.0.0 but it does not have these changes included. Any help is appreciated.

@jcesarmobile
Copy link
Member Author

Please, don’t ping me, I’m already subscribed to the issue, so you don’t need to ping me.

5.0.0 was released on Feb 10, 2021, this was merged on Jun 11, 2021l so no, it’s not included.

you can always install from the GitHub url like cordova plugin add https://github.com/apache/cordova-plugin-inappbrowser and get latest changes.

@ShahinBelim
Copy link

any plans on publishing these latest changes in near future?

@NiklasMerz
Copy link
Member

I will start the discussion about a new relase on the mailing list now. I think I can take care of a new release in the near future, but please be patient for a little longer.

@apache apache locked as resolved and limited conversation to collaborators Nov 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't open mailto: / tel: / sms: links in IAB under iOS: URL not supported
4 participants