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

iOS CDVWKInAppBrowser evaluateJavascript method randomly gets blocked on ios 12 #341

Merged
merged 2 commits into from
Nov 19, 2018
Merged

Conversation

jonathanli2
Copy link
Contributor

@jonathanli2 jonathanli2 commented Nov 2, 2018

Platforms affected

iOS

What does this PR do?

fix issue #340 iOS CDVWKInAppBrowser evaluateJavascript method randomly gets blocked on iOS 12

What testing has been done on this change?

manual testing

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

@jonathanli2
Copy link
Contributor Author

This is a blocking issue, could we review and merge the change soon?

Thanks
Jonathan

@janpio
Copy link
Member

janpio commented Nov 19, 2018

Will this also work in older iOS versions <12?

@dpa99c
Copy link
Contributor

dpa99c commented Nov 19, 2018

@janpio yes, WKWebView's evaluateJavascript and the associated addScriptMessageHandler used to return the async result via the postMessage API have both been present since iOS 8.

The changes in the PR by @jonathanli2 simply remove the redundant return of the synchronous result which is never used in the WKWebView implementation (my bad for not removing it originally).

@janpio janpio merged commit 978b147 into apache:master Nov 19, 2018
@janpio
Copy link
Member

janpio commented Nov 19, 2018

Thanks, I don't really know enough about iOS and Obj-C to make decisions like this merge myself - but as it's "your code" I went ahead @dpa99c :)

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

Successfully merging this pull request may close these issues.

3 participants