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.

Sign up for GitHub

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

Jump to bottom

window.handleOpenURL is triggered in WKWebView for system browser calls #652

Closed
3 tasks done
j--w opened this issue Mar 24, 2020 · 3 comments · Fixed by #654
Closed
3 tasks done

window.handleOpenURL is triggered in WKWebView for system browser calls #652

j--w opened this issue Mar 24, 2020 · 3 comments · Fixed by #654
Labels
bug has-pr platform: ios

Comments

@j--w
Copy link
Contributor

j--w commented Mar 24, 2020 •
edited
Loading

Bug Report

When triggering a url to open in the _system browser through IAB with WKWebView window.handleOpenURL is called. If using UIWebView this does not happen. This appears to be a regression as this issue was fixed with this PR but it appears the check never made it into CDVWKInAppBrowser.m

Problem

I believe there should be a check around the postNotification eg: if ([[UIApplication sharedApplication] openURL:url] == NO)

What is expected to happen?

window.handleOpenURL is not called

What does actually happen?

window.handleOpenURL is not called

Information

Command or Code

I created a repo to reproduce
https://github.com/j--w/iab-handleopenurl-bug/blob/master/www/js/index.js#L30

Environment, Platform, Device

Version information

Cordova CLI: 7.1.0
Cordova IOS: 5.1.1
XCode 11.2.1
Cordova InAppBrowser 3.2.0
OSX

Checklist

  • I searched for existing GitHub issues
  • I updated all Cordova tooling to most recent version
  • I included all the necessary information above
@timbru31
Copy link
Member

PRs very welcome and thanks for following the issue template and providing a repro!

@j--w
Copy link
Contributor Author

j--w commented Mar 24, 2020

I'll work on a PR for this now

j--w pushed a commit to j--w/cordova-plugin-inappbrowser that referenced this issue Mar 24, 2020
[apacheGH-652] add check for openInSystem postNotification
97677a6
@j--w j--w mentioned this issue Mar 25, 2020
Merged
5 tasks
timbru31 pushed a commit that referenced this issue Mar 30, 2020
@j--w
[GH-652] add check for openInSystem postNotification (#654)
11ba4c4 
Co-authored-by: Jim Wright <[email protected]>
expertanswerz pushed a commit to expertanswerz/cordova-plugin-inappbrowser that referenced this issue Apr 9, 2020
@ChanchleshSuryawanshi
Merge branch 'master' into 3.2.x
54528fb 
* master:
  docs: replaces outdated transition and presentation style links (apache#662)
  chore: remove deprecated orientation methods (apache#666)
  Fix incorrect TypeScript typings (apache#515)
  [apacheGH-652] add check for openInSystem postNotification (apache#654)
  ci: updates Node.js versions (apache#659)
  chore(npm): improve ignore list (apache#658)
  fix(android): Reset lefttoright if not set (apache#442)
  [android] Correcting the documentation regarding lefttoright opt… (apache#648)
  (android) Added option to turn on/off fullscreen mode in Android (apache#634)
  Android apacheGH-470 InAppBrowser: java.lang.IllegalArgumentException (apache#616)
  breaking(ios): remove UIWebView (apache#635)
  chore(release): 3.2.1-dev
@rolinger
Copy link

rolinger commented Oct 22, 2020 •
edited
Loading

@j--w @timbru31 - looks like this bug is back. I am on inappbrowser ~4.0.0 and cordova-plugin-ionic-webview ~5.0.0 and this bug just happened to me. After further inspection, this bug still persists if you put in a bad URL. I was trying to link to geo:?q=82.12334,88,54321 and it was causing a break on this same bug. But I realized iOS uses maps:, not geo - when I changed it everything worked fine. I tried a few more bad urls and each time it caused the same error above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug has-pr platform: ios
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[GH-652] add check for openInSystem postNotification j--w/cordova-plugin-inappbrowser
3 participants
@timbru31 @j--w @rolinger