You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by "GitToTheHub (via GitHub)" <gi...@apache.org> on 2025/10/10 14:52:00 UTC
[PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
GitToTheHub opened a new pull request, #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190
- Refactored getTopPresentedViewController to use the active UIWindowScene for keyWindow selection, improving compatibility with iOS multi-window environments.
- Using `[UIApplication sharedApplication].keyWindow` is deprecated since iOS 13
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### Platforms affected
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
### Description
<!-- Describe your changes in detail -->
### Testing
<!-- Please describe in detail how you tested your changes. -->
### 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](https://help.github.com/articles/closing-issues-using-keywords/))
- [ ] I've updated the documentation if necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "jcesarmobile (via GitHub)" <gi...@apache.org>.
jcesarmobile commented on code in PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#discussion_r2422966483
##########
src/ios/CDVNotification.m:
##########
@@ -149,14 +149,39 @@ - (void)beep:(CDVInvokedUrlCommand*)command
}
-(UIViewController *)getTopPresentedViewController {
+ UIWindow *keyWindow = nil;
+
+ if (@available(iOS 13.0, *)) {
+ // iOS 13+ approach - get the first active window scene
+ // Since iOS 13, Apple introduced UIScene and multiple window support.
+ // The deprecated keyWindow property doesn't work reliably with multiple scenes
+ // as it returns a key window across all connected scenes, which can be from
+ // different app instances or windows. We need to find the active foreground
+ // scene to get the correct window for presenting our alert.
+ for (UIWindowScene *windowScene in [UIApplication sharedApplication].connectedScenes) {
+ if (windowScene.activationState == UISceneActivationStateForegroundActive) {
+ keyWindow = windowScene.windows.firstObject;
+ break;
+ }
+ }
+ } else {
+ // Fallback for iOS 11-12
Review Comment:
The plugin technically supports iOS 8+ (and dropping iOS 7 and older is what caused the still unreleased 3.x version bump), so better just say "fallback for older versions" instead of giving specifics
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#issuecomment-3393419790
Is somewhere documented, which cordova-ios supports which minimum iOS version?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on code in PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#discussion_r2423350539
##########
src/ios/CDVNotification.m:
##########
@@ -149,14 +149,39 @@ - (void)beep:(CDVInvokedUrlCommand*)command
}
-(UIViewController *)getTopPresentedViewController {
+ UIWindow *keyWindow = nil;
+
+ if (@available(iOS 13.0, *)) {
+ // iOS 13+ approach - get the first active window scene
+ // Since iOS 13, Apple introduced UIScene and multiple window support.
+ // The deprecated keyWindow property doesn't work reliably with multiple scenes
+ // as it returns a key window across all connected scenes, which can be from
+ // different app instances or windows. We need to find the active foreground
+ // scene to get the correct window for presenting our alert.
+ for (UIWindowScene *windowScene in [UIApplication sharedApplication].connectedScenes) {
+ if (windowScene.activationState == UISceneActivationStateForegroundActive) {
+ keyWindow = windowScene.windows.firstObject;
+ break;
+ }
+ }
+ } else {
+ // Fallback for iOS 11-12
Review Comment:
> Not everybody submit apps to the Apple App Store, so in those cases there are no Xcode restrictions and people can have real devices to test with those older iOS versions.
Do this really do people? Do know real examples, or this just a guess? I mean also with the old os versions.
Thanks for the hint with the dev list, I will try this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on code in PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#discussion_r2423026176
##########
src/ios/CDVNotification.m:
##########
@@ -149,14 +149,39 @@ - (void)beep:(CDVInvokedUrlCommand*)command
}
-(UIViewController *)getTopPresentedViewController {
+ UIWindow *keyWindow = nil;
+
+ if (@available(iOS 13.0, *)) {
+ // iOS 13+ approach - get the first active window scene
+ // Since iOS 13, Apple introduced UIScene and multiple window support.
+ // The deprecated keyWindow property doesn't work reliably with multiple scenes
+ // as it returns a key window across all connected scenes, which can be from
+ // different app instances or windows. We need to find the active foreground
+ // scene to get the correct window for presenting our alert.
+ for (UIWindowScene *windowScene in [UIApplication sharedApplication].connectedScenes) {
+ if (windowScene.activationState == UISceneActivationStateForegroundActive) {
+ keyWindow = windowScene.windows.firstObject;
+ break;
+ }
+ }
+ } else {
+ // Fallback for iOS 11-12
Review Comment:
cordova-ios 6 is the first one, which supports iOS 11. So we should support minimum this cordova-ios version. But for cordova-ios 8.0.0, we should raise the minimum iOS version to 15, because you can't test your app on older OSs. XCode doesn't let you install older simulators and according to ChatGPT, you can't install with the oldest supported XCode 16 your app on a device which has an iOS version older than 15. I started a discussion for this here: https://github.com/apache/cordova/discussions/566
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on code in PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#discussion_r2422968056
##########
src/ios/CDVNotification.m:
##########
@@ -149,14 +149,39 @@ - (void)beep:(CDVInvokedUrlCommand*)command
}
-(UIViewController *)getTopPresentedViewController {
+ UIWindow *keyWindow = nil;
+
+ if (@available(iOS 13.0, *)) {
+ // iOS 13+ approach - get the first active window scene
+ // Since iOS 13, Apple introduced UIScene and multiple window support.
+ // The deprecated keyWindow property doesn't work reliably with multiple scenes
+ // as it returns a key window across all connected scenes, which can be from
+ // different app instances or windows. We need to find the active foreground
+ // scene to get the correct window for presenting our alert.
+ for (UIWindowScene *windowScene in [UIApplication sharedApplication].connectedScenes) {
+ if (windowScene.activationState == UISceneActivationStateForegroundActive) {
+ keyWindow = windowScene.windows.firstObject;
+ break;
+ }
+ }
+ } else {
+ // Fallback for iOS 11-12
Review Comment:
I don't understand, where the minimum iOS version is configured. I looked in `package.json`, but found only this:
```json
"cordovaDependencies": {
"3.0.0": {
"cordova": ">100"
}
```
What dos `"cordova": ">100"` mean?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on code in PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#discussion_r2423476492
##########
src/ios/CDVNotification.m:
##########
@@ -149,14 +149,39 @@ - (void)beep:(CDVInvokedUrlCommand*)command
}
-(UIViewController *)getTopPresentedViewController {
+ UIWindow *keyWindow = nil;
+
+ if (@available(iOS 13.0, *)) {
+ // iOS 13+ approach - get the first active window scene
+ // Since iOS 13, Apple introduced UIScene and multiple window support.
+ // The deprecated keyWindow property doesn't work reliably with multiple scenes
+ // as it returns a key window across all connected scenes, which can be from
+ // different app instances or windows. We need to find the active foreground
+ // scene to get the correct window for presenting our alert.
+ for (UIWindowScene *windowScene in [UIApplication sharedApplication].connectedScenes) {
+ if (windowScene.activationState == UISceneActivationStateForegroundActive) {
+ keyWindow = windowScene.windows.firstObject;
+ break;
+ }
+ }
+ } else {
+ // Fallback for iOS 11-12
Review Comment:
> The plugin technically supports iOS 8+ (and dropping iOS 7 and older is what caused the still unreleased 3.x version bump), so better just say "fallback for older versions" instead of giving specifics
I fixed it by not mention any version
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "jcesarmobile (via GitHub)" <gi...@apache.org>.
jcesarmobile commented on PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#issuecomment-3393417630
latest stable release of cordova-ios supports iOS 11+, if we want to drop support for iOS < 13 for the plugin we should wait until cordova-ios 8 is released and add engine tags in plugin.xml and package.json to require that version, otherwise we should keep the old code with version checks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "jcesarmobile (via GitHub)" <gi...@apache.org>.
jcesarmobile commented on code in PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#discussion_r2422983929
##########
src/ios/CDVNotification.m:
##########
@@ -149,14 +149,39 @@ - (void)beep:(CDVInvokedUrlCommand*)command
}
-(UIViewController *)getTopPresentedViewController {
+ UIWindow *keyWindow = nil;
+
+ if (@available(iOS 13.0, *)) {
+ // iOS 13+ approach - get the first active window scene
+ // Since iOS 13, Apple introduced UIScene and multiple window support.
+ // The deprecated keyWindow property doesn't work reliably with multiple scenes
+ // as it returns a key window across all connected scenes, which can be from
+ // different app instances or windows. We need to find the active foreground
+ // scene to get the correct window for presenting our alert.
+ for (UIWindowScene *windowScene in [UIApplication sharedApplication].connectedScenes) {
+ if (windowScene.activationState == UISceneActivationStateForegroundActive) {
+ keyWindow = windowScene.windows.firstObject;
+ break;
+ }
+ }
+ } else {
+ // Fallback for iOS 11-12
Review Comment:
It's not specified anywhere at the moment, when the version was bumped to 3.0.0-dev and iOS 7 and older code was removed we should have added a cordova-ios >= 4 and the > 100 should have been moved to a new 4.0.0 entry.
The > 100 was supposed to be a protective measure to prevent installing a version when a major bump happens or something, that has only caused issues as we usually forget to update it and it prevents the installing on the version of the plugin.
you can read about the proposal here
https://lists.apache.org/thread/lfnxoss879yqw0rbljqycms7r0g6vgy7
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on code in PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#discussion_r2423745261
##########
src/ios/CDVNotification.m:
##########
@@ -149,14 +149,39 @@ - (void)beep:(CDVInvokedUrlCommand*)command
}
-(UIViewController *)getTopPresentedViewController {
+ UIWindow *keyWindow = nil;
+
+ if (@available(iOS 13.0, *)) {
+ // iOS 13+ approach - get the first active window scene
+ // Since iOS 13, Apple introduced UIScene and multiple window support.
+ // The deprecated keyWindow property doesn't work reliably with multiple scenes
+ // as it returns a key window across all connected scenes, which can be from
+ // different app instances or windows. We need to find the active foreground
+ // scene to get the correct window for presenting our alert.
+ for (UIWindowScene *windowScene in [UIApplication sharedApplication].connectedScenes) {
+ if (windowScene.activationState == UISceneActivationStateForegroundActive) {
+ keyWindow = windowScene.windows.firstObject;
+ break;
+ }
+ }
+ } else {
+ // Fallback for iOS 11-12
Review Comment:
Ok thanks for the information. So it's better to support the older ones, though they are not distributable to the app store.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#issuecomment-3393421113
> Is somewhere documented, which cordova-ios supports which minimum iOS version?
I see it's in the [iOS Platform Guide](https://cordova.apache.org/docs/en/12.x-2025.01/guide/platforms/ios/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "jcesarmobile (via GitHub)" <gi...@apache.org>.
jcesarmobile commented on code in PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#discussion_r2423081762
##########
src/ios/CDVNotification.m:
##########
@@ -149,14 +149,39 @@ - (void)beep:(CDVInvokedUrlCommand*)command
}
-(UIViewController *)getTopPresentedViewController {
+ UIWindow *keyWindow = nil;
+
+ if (@available(iOS 13.0, *)) {
+ // iOS 13+ approach - get the first active window scene
+ // Since iOS 13, Apple introduced UIScene and multiple window support.
+ // The deprecated keyWindow property doesn't work reliably with multiple scenes
+ // as it returns a key window across all connected scenes, which can be from
+ // different app instances or windows. We need to find the active foreground
+ // scene to get the correct window for presenting our alert.
+ for (UIWindowScene *windowScene in [UIApplication sharedApplication].connectedScenes) {
+ if (windowScene.activationState == UISceneActivationStateForegroundActive) {
+ keyWindow = windowScene.windows.firstObject;
+ break;
+ }
+ }
+ } else {
+ // Fallback for iOS 11-12
Review Comment:
The latest release of the plugin supports all iOS versions, the unreleased 3.0.0 would support iOS 8+ as it removed old UIAlertView code, and the Cordova-iOS version that supports iOS 8+ is 4.0.0. We could require Cordova-iOS 6 if we wanted, but there is no technical reason to do so as the code you added will work on older versions too.
Not everybody submit apps to the Apple App Store, so in those cases there are no Xcode restrictions and people can have real devices to test with those older iOS versions.
About the min iOS version discussion, I think you should send a mail to the Apache dev list, GitHub discussions is more for user questions, since this is a formal discussion from a contributor it should go to the dev list, but some other contributors have been opposed to raising the deployment target, it has already been raised to 13 and was not easy to get.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#issuecomment-3393425738
> latest stable release of cordova-ios supports iOS 11+, if we want to drop support for iOS < 13 for the plugin we should wait until cordova-ios 8 is released and add engine tags in plugin.xml and package.json to require that version, otherwise we should keep the old code with version checks.
You are correct, I forgot to support minimum iOS 11. It corrected this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] iOS: Update top view controller retrieval for iOS multi-window [cordova-plugin-dialogs]
Posted by "jcesarmobile (via GitHub)" <gi...@apache.org>.
jcesarmobile commented on code in PR #190:
URL: https://github.com/apache/cordova-plugin-dialogs/pull/190#discussion_r2423534054
##########
src/ios/CDVNotification.m:
##########
@@ -149,14 +149,39 @@ - (void)beep:(CDVInvokedUrlCommand*)command
}
-(UIViewController *)getTopPresentedViewController {
+ UIWindow *keyWindow = nil;
+
+ if (@available(iOS 13.0, *)) {
+ // iOS 13+ approach - get the first active window scene
+ // Since iOS 13, Apple introduced UIScene and multiple window support.
+ // The deprecated keyWindow property doesn't work reliably with multiple scenes
+ // as it returns a key window across all connected scenes, which can be from
+ // different app instances or windows. We need to find the active foreground
+ // scene to get the correct window for presenting our alert.
+ for (UIWindowScene *windowScene in [UIApplication sharedApplication].connectedScenes) {
+ if (windowScene.activationState == UISceneActivationStateForegroundActive) {
+ keyWindow = windowScene.windows.firstObject;
+ break;
+ }
+ }
+ } else {
+ // Fallback for iOS 11-12
Review Comment:
> > Not everybody submit apps to the Apple App Store, so in those cases there are no Xcode restrictions and people can have real devices to test with those older iOS versions.
>
> Do this really do people? Do know real examples, or this just a guess? I mean also with the old os versions.
>
> Thanks for the hint with the dev list, I will try this.
Yeah, since mac computers are expensive there are people not being able to buy them so often and use older versions with older Xcode versions since Apple also requires certain OS versions for using latest Xcode versions.
Apple allows "in house" distribution of apps for enterprise customers that since are hosted on the enterprise servers don't have Xcode version restrictions.
Also allows installing the apps from Xcode directly, even without a paid developer license nowadays.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org