Page MenuHomePhabricator
Differential D13265

Fix broken collapsable notifs in badge only threads on iOS
ClosedPublic
Actions

Authored by marcin on Mon, Sep 9, 2:09 AM.
Tags
None
Referenced Files
F2753957: D13265.id43971.diff
Wed, Sep 18, 6:32 PM
Unknown Object (File)
Tue, Sep 17, 1:31 AM
Unknown Object (File)
Sun, Sep 15, 10:01 PM
Unknown Object (File)
Sun, Sep 15, 11:00 AM
Unknown Object (File)
Sun, Sep 15, 4:39 AM
Unknown Object (File)
Sun, Sep 15, 4:02 AM
Unknown Object (File)
Sat, Sep 14, 10:14 PM
Unknown Object (File)
Sat, Sep 14, 3:01 PM
View All 21 Files
Subscribers
None

Details

Reviewers
tomek
kamil
ashoat
Commits
rCOMM6b3f5d4e2047: Fix broken collapsable notifs in badge only threads on iOS
Summary

When user receives notification with collapse id in badge only thread it will result in notification with "ENCRYPTED" body. This diff fixes that.

Test Plan
  1. Prior to this diff every collapsable notif (image, change settings etc.) resulted in ENCRYPTED notifs in a badge-only thread on iOS.
  2. After this diff the issue is fixed.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

marcin created this revision.Mon, Sep 9, 2:09 AM
Harbormaster completed remote builds in B31482: Diff 43965.Mon, Sep 9, 2:26 AM
marcin requested review of this revision.Mon, Sep 9, 2:26 AM
tomek accepted this revision.Mon, Sep 9, 2:50 AM
This revision is now accepted and ready to land.Mon, Sep 9, 2:50 AM
marcin updated this revision to Diff 43968.Mon, Sep 9, 2:55 AM

Rebase before landing

This revision was landed with ongoing or failed builds.Mon, Sep 9, 3:09 AM
Closed by commit rCOMM6b3f5d4e2047: Fix broken collapsable notifs in badge only threads on iOS (authored by marcin). · Explain Why
This revision was automatically updated to reflect the committed changes.
marcin added a commit: rCOMM6b3f5d4e2047: Fix broken collapsable notifs in badge only threads on iOS.
Harbormaster completed remote builds in B31484: Diff 43968.Mon, Sep 9, 3:20 AM
ashoat added a comment.Mon, Sep 9, 3:41 AM

Just wondering, what codepath led to the ENCRYPTED notif? Curious why we saw that instead of Obj-C exception or C++ exception

marcin added a comment.Mon, Sep 9, 6:11 AM
In D13265#373994, @ashoat wrote:

Just wondering, what codepath led to the ENCRYPTED notif? Curious why we saw that instead of Obj-C exception or C++ exception

The code path flows like this:

  1. First the keyserver inserts ENCRYPTED body to aps dictionary on encrypted badge only notification: https://github.com/CommE2E/comm/blob/5e6732ede2db1aed9e4f2cb15db249a87e643aa3/keyserver/src/push/crypto.js#L88
  2. The NSE decrypts notification. There is no body property in decryptedPayload so it does not overwrite the body property that is in aps dictionary: https://github.com/CommE2E/comm/blob/5e6732ede2db1aed9e4f2cb15db249a87e643aa3/native/ios/NotificationService/NotificationService.mm#L679-L687
  3. Then the NSE sees that this notification has collapse key so it creates local notification that has the same body as the original notification (which is ENCRYPTED): https://github.com/CommE2E/comm/blob/5e6732ede2db1aed9e4f2cb15db249a87e643aa3/native/ios/NotificationService/NotificationService.mm#L251
ashoat added a comment.Mon, Sep 9, 8:32 AM

Thanks for explaining, @marcin!

Revision Contents
Changeset List

PathSize
native/
ios/
NotificationService/
3 lines

Diff 43971

View Options

native/ios/NotificationService/NotificationService.mm

Loading...