-
Notifications
You must be signed in to change notification settings - Fork 114
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 extension crash on iOS 13 #69
Conversation
@j3k0 As this only works in Xcode 11 and newer how do we implement this? Use build flags? I am not that familiar with this. New major version? Do you have time to look into this? We also need version checks for iOS 13. At least there is a warning for that. But this new code works for me on iOS 12.4. That doesn't really make sense to me. And I don't really know how to make this version check in a good way. |
This is my attempt to check version. There has to be a cleaner version. |
@EternallLight Would you like to review and/or test this? |
Hey Niklas,
Looked into the code, this should be fine (I think the `@available`
thing requires XCode 9, this is fine, Apple is quickly requiring devs to
build against the latest SDK to get apps approved, so all devs must be
mostly up-to-date). I'm sorry I don't have much time to test this code
though (no test bed in place). I'm super busy at least till the end of
the month.
Feel free to merge as you please, you're really the main maintainer of
this project.
JC
…On Mon, Sep 23, 2019 at 04:52:05AM -0700, Niklas Merz wrote:
This is my attempt to check version. There has to be a cleaner version.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#69 (comment)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works for me @ ios13 and xcode 11.0
Thank you for testing. |
Hi, i tested the fix on iOS 13 as well as on iOS 12.4. Works without any issues 👍 |
@j3k0 I am thinking about doing a new release with this fix. Since this is a breaking change (requiring Xcode 11), should we do a major relase (2.0)? |
Hey @NiklasMerz, happy new year :) I agree, doing a major release seems appropriate. Do you have |
#72 seems good to include too in the new release. |
Happy new year. Yes I have NPM permission and will do the release right now. |
Version 2.0 is released. https://github.com/j3k0/cordova-plugin-openwith/releases/tag/v2.0.0 I just did a mistake, I had to revert. I checked in package-lock.json that's why the dependabot PR's opened. @j3k0 Should we check package-lock.json in to git and update the dependencies? |
@NiklasMerz as far as I know. |
Good to know. Then we could add it to gitignore |
* Fix extension crash on iOS 13 * Fix UISceneOpenExternalURLOptions init * iOS 13 version check
This reverts commit b2830f9.
into j3k0-master * 'master' of https://github.com/j3k0/cordova-plugin-openwith: Fix missing quotes Change code sign indentity and style Fix async call Added a copy files to after install hook IOS : Fix __BUNDLE_IDENTIFIER__ to prevent double suffix IOS : Add ShareExtension-Entitlements.plist to package.json IOS : Remove "iOS Setup" section IOS : Fix App Group creation and fix Share Extension Bundle Identifier IOS : Fix getCordovaParameter() to use package.json Because after_prepare hook does not pass installation variables in process.argv IOS : Move hooks on before and after prepare This allows ShareExtension to be created and updated in any case 2.0.0 Bump version 2.0.0 change hook triggering iosAddTarget.js. Fixes the target not being added to xcode project. (j3k0#72) Fix iOS build flags (j3k0#84) Fix extension crash on iOS 13 (j3k0#69) 1.3.0 Set version in plugin.xml
* j3k0-master: Fix missing quotes Change code sign indentity and style Fix async call Added a copy files to after install hook IOS : Fix __BUNDLE_IDENTIFIER__ to prevent double suffix IOS : Add ShareExtension-Entitlements.plist to package.json IOS : Remove "iOS Setup" section IOS : Fix App Group creation and fix Share Extension Bundle Identifier IOS : Fix getCordovaParameter() to use package.json Because after_prepare hook does not pass installation variables in process.argv IOS : Move hooks on before and after prepare This allows ShareExtension to be created and updated in any case 2.0.0 Bump version 2.0.0 change hook triggering iosAddTarget.js. Fixes the target not being added to xcode project. (j3k0#72) Fix iOS build flags (j3k0#84) Fix extension crash on iOS 13 (j3k0#69) 1.3.0 Set version in plugin.xml
Changing the options seems to fix the crash on iOS 13. Then strange thing is that this seems to work on iOS 12 aswell when built with Xcode 11. Working with
@available
and the old option did not work for me and now it shows warnings because the new optiosn are on iOS 13 only. No idea why.This "fix" needs Xcode 11 so we should wait before merging this to a new version.
Fixes #67