v0.10.0; multiple Android and iOS fixes

[fquirin]

Latest v0.10.0 is a merge of @timkellypa, @bhandaribhumin (with changes by @powowbox) and @fquirin.

Notable changes:

• Fixed Android 12 bug to make notifications clickable (@powobox) by replacing the action handler services with activities (avoid notification trampoline restrictions)
• Added missing 'PendingIntent.FLAG_MUTABLE' and fixed gradle
• Guard against webview crash
• Add thread identifier property
• Delete Alarms when intent is deleted
• Not calling delegate events if nil or if we're consuming the notification
• Android 13 POST_NOTIFICATIONS permission and runtime popup added
• New interfaces to ask for / register permissions required to schedule local notifications
• ... (see CHANGELOG)

[GitToTheHub]

@fquirin There is a bug in the method "dummyNotifications". It creates a channel "NOTIFICATION_CHANNEL_NAME". The channel name and id is not configurable.

[GitToTheHub]

@fquirin You could add in the notable changes for "fixed gradle" that "compile" is replaced by "implementation" for the notifications plugin and for the badge-plugin by using a fixed fork. It would be cool, if you would extend the notable changes by adding "iOS" or "Android". So it's clear, to which platform a change is related.

[dpalou]

This PR looks great, thank you for your work on this! But I think it's missing something. The plugin.xml file was modified to include the POST_NOTIFICATIONS permission, but then the code is not requesting the permission. AFAIK apps compiled with targetSdk=33 and installed in Android 13 have the notifications disabled by default and you need to request the user permission to enable them, something like:

PermissionHelper.requestPermissions(this, REQUEST_POST_NOTIFICATIONS, new String[]{Manifest.permission.POST_NOTIFICATIONS});

Right now the request method doesn't do that, it only checks if the permission is granted or not.

[fquirin]

Hi @dpalou ,

that is unfortunately correct. I'll have to fix this sooner or later in my own app, but in the meantime you can use this:

cordova.plugins.notification.local.setDummyNotifications();

A quote from the README:

This method allows user to trigger runtime permission for android 13

I'm currently considering this plugin to handle permissions (haven't tried it yet though)

[GitToTheHub]

@fquirin Could you maybe fix your pr with the changes @dpalou suggested or otherwise? This would be really great.

[fquirin]

I'm currently hands deep in another project that I have to finish first. Maybe I'll find time to review the comments above and add a new permission request interface next month, but I can't guarantee it, sorry. Btw, if you send a PR to the 'pr-katzer' branch in my fork it should appear here as well (when I accept it ofc) ;-)

[GitToTheHub]

No problem, i only asked you, if you maybe had time for it. If you would do it in a month, it would be cool anyway. I want to merge the pr's to this project, so it's functional again. I'm just missing the permission thing for android 13. Thank you for the advise with your fork. If i find time, i know where i can fix this :)

[fquirin]

Oh, I didn't notice that you can merge PRs here 😃, did @katzer hand it over to you?

[GitToTheHub]

Yes I asked him if I can maintain this project :)

[powowbox]

This PR includes some code I did to make local notifications work on android 13. The version you use have <preference name="AndroidLaunchMode" value="singleInstance"/> in the config.xml file. It's better to avoid this because it can cause issues with the cordova camera plugin. See https://stackoverflow.com/questions/10831562/camera-always-returns-resultcode-as-0.

In the last version of my fork https://github.com/powowbox/cordova-plugin-local-notification-12 I made changes to avoid the use of "single instance". It should be better to include them, they are summarized at the top of the README in sections "Purpose of this fork" and "Important notice".

[fquirin]

@GitToTheHub I've done some smaller updates, mostly regarding the comments you've made. I did not touch the permission stuff yet, because the current implementation looks confusing and an improvement will require a lot of testing and a better plan to handle it.

@powowbox I've tried your changes with my app and as long as I stay on singleInstance it works, but when I switch back to singleTask (I've used this in the old version of my app) I seem to loose notification click-events that are triggered while the app is open 🤔. I get the event when I close the app before. Any idea what might cause this?

[fquirin]

@dpalou I'm assuming PermissionHelper is org.apache.cordova.PermissionHelper right?

• Do we have to put an Android version check around it?
• Should we use PermissionHelper.hasPermission() as well?
• How do we prevent that the user will be asked over and over again if he refuses?
• Is there a callback like onRequestPermissionsResult?

[GitToTheHub]

@fquirin Thank you for your changes. There is a PR from @timkellypa "Android 10 updates" #1891 Should we first merge that PR and after this your PR? Or close that PR and merge only your PR?

[fquirin]

I started from his branch, so I'm assuming all of this changes should be included in my PR. At least my branch says I'm not behind any of his changes.

[GitToTheHub]

So I will first merge his PR for better documentation. I think then it's more clear, what this PR will do. That will not causes problems for you, or?

[fquirin]

hope not, we'll see ^^

[GitToTheHub]

Ok :)

[powowbox]

@fquirin I will try to make some tests this week.

[GitToTheHub]

I merged the android 10 updates from @timkellypa and 2 gradle pr's. It seems everything ok for you. I communicated with the author of this plugin, if he can make a new release of cordova-plugin-badge. So if he did this, we can link this pr to the original plugin back.

[fquirin]

I added a comment so we don't forget :-) and resolved the README conflict

[GitToTheHub]

Very good. Do you know, why we can see still the Android 10 updates from @timkellypa in your pr though i merged them already?

[fquirin]

Do you mean the commit from 'Jan 30, 2021'? I think that is just some branch merging, idk, but it seems to show no actual changes 🤔. All the 2020 commits seem to have disappeared (as expected).

[GitToTheHub]

Yeah i see this, they are disappeared. Very good, I'm happy :)

[dpalou]

Hi @fquirin ,

I've barely developed Android native code in 8 years, so I'm probably not the best person to ask for advice ^^' That's why I didn't submit a PR to your branch.

Yes, PermissionHelper is org.apache.cordova.PermissionHelper. I saw it used in cordova-plugin-camera. They use hasPermission too, I don't know if it should be used here too or using getNotCompMgr().areNotificationsEnabled() is enough.

I don't know if an Android version check is needed or not, maybe in old versions the hasPermission function always returns true so it isn't needed, I didn't try.

I think that if the user rejects the permission then the requestPermission function will not prompt him again, it will act as if the user denied the permission again. But I cannot guarantee it without some testing.

About onRequestPermissionsResult, the CordovaPlugin class (the one LocalNotification inherits from) already implements this callback and can be overriden. The first parameter passed to requestPermissions should be the class that implements this callback.

I'm currently with another project too, if I have time in the following weeks I can try to perform a few more tests.

Thanks again for your work, and thanks @GitToTheHub too for assuming the maintenance of this plugin :)

Cheers, Dani

[GitToTheHub]

Thanks again for your work, and thanks @GitToTheHub too for assuming the maintenance of this plugin :)

Thank you :)

[bartvanvelden]

Great to see the changes proposed here. What needs to be done to have this finalised?

[GitToTheHub]

First I need access to cordova-plugin-badge from the author, to make a new release. Then we need the fixes for this pr from the other contributors.

[dpalou]

Just for the record, this is the code I've ended up using in our fork to request the POST_NOTIFICATIONS runtime permission if needed. Maybe the same can be achieved using the setDummyNotifications method included in this PR, I don't know.

If the permission is already granted or device is Android 12-, nothing will happen. If it isn't, then the user will be asked when:

• The JS code of the app calls requestPermission.
• The JS code of the app calls schedule or update with skipPermission != true.

In that case, the user will be able to allow or deny the permission. If it's denied, the next time any of the functions above is called the user will be asked a second time. If it's denied again, then it will be denied forever, the user won't be asked again.

I'm not an Android native developer so I might have done something wrong, the first tests were ok but we still need to do some exhaustive testing about this. Feel free to cherry-pick this commit in the PR if you want :)

[GitToTheHub]

@fquirin Do you think the code from @dpalou could be helpful for you, to unify the requestPermission thing? I already talked with @katzer and he will release a new release for cordova-plugin-badge the next time.

[fquirin]

Do you think the code from @dpalou could be helpful for you, to unify the requestPermission thing?

I think so, the code looks pretty straight forward. Currently I'm more concerned about the issue with click-events and singleTask or respectively the camera and singleInstance (https://github.com/katzer/cordova-plugin-local-notifications/pull/2000#issuecomment-1475294422). This will require A LOT of testing.

I already talked with @katzer and he will release a new release for cordova-plugin-badge the next time.

Top 👍