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

Push Payload Changes #114

Merged
merged 2 commits into from
May 7, 2020
Merged

Push Payload Changes #114

merged 2 commits into from
May 7, 2020

Conversation

ismailgulek
Copy link
Contributor

Change content-available to mutable-content and also add event_id in payload if provided. This is intended to enable modifying push notification content on the client side (iOS). For details:

element-hq/element-ios#2714

Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an APNS expert, but this doesn't sound backward-compatible to me. Could you please define the new property as well as the existing one, not instead?

@ismailgulek
Copy link
Contributor Author

@babolivier thanks for the review. I'll try to explain the change as far as i can.

content-available key is used to make a notification silent, when the key is set to 1, alert, badge and sound keys should be omitted. Detailed info about silent notifications is here.

mutable-content key is used to make a notification modifiable in client side. We will use this mechanism as a replacement of VoIP pushes and our intention is to modify notification content (e.g. decrypt event) and if we fail on this, we'll still have some generic content to show (e.g. "Somebody sent you a message"). Detailed info about modifying notification content is here.

Actually our usage of content-available key was useless because we were also setting the alert and badge keys. So these notifications were not silent. Indeed, we were not using regular notifications at all, but VoIP pushes.

Detailed info about payload keys can be found here.

@babolivier
Copy link
Contributor

babolivier commented May 7, 2020

@ismailgulek Right, so neither content-available nor mutable-content are actually passed on to clients?

@ismailgulek
Copy link
Contributor Author

ismailgulek commented May 7, 2020

@ismailgulek Right, so neither content-available nor mutable-content are actually passed on to clients?

No, they are both passed. But they don't help us at the moment, because we're using VoIP pushes. They will be important with our new development.

@babolivier
Copy link
Contributor

No, they are both passed

Right, my point is that Riot isn't the only mobile client that's using push so we should care about backwards compatibility in case one actually does something with these parameters.

@ismailgulek
Copy link
Contributor Author

ismailgulek commented May 7, 2020

No, they are both passed

Right, my point is that Riot isn't the only mobile client that's using push so we should care about backwards compatibility in case one actually does something with these parameters.

It should not affect any other clients, as they already should be using VoIP pushes. Even if they don't, as i stated earlier, it's useless now.

@babolivier babolivier dismissed their stale review May 7, 2020 12:27

This concern has been evaluated and we consider it unlikely to backfire on us in the future.

@babolivier babolivier merged commit 9a1e3c8 into master May 7, 2020
@babolivier babolivier deleted the riot_2714 branch May 7, 2020 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants