Skip to content

Commit 4bc7ecc

Browse files
committed
ios: Remove didReceiveRemoteNotification method override in AppDelegate
Apple's documentation for this UIApplication method ( https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1623013-application ) says at the top: > Tells the app that a remote notification arrived that indicates > there is data to be fetched. From experimentation, it turns out that the qualifier "that indicates…" is quite important! It seems to mean that the method won't be called except for "background update notifications". That's a special kind of notification that isn't mentioned by name in that doc, but is discussed in a section of a different doc, on creating remote notification payloads: https://developer.apple.com/library/archive/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/CreatingtheNotificationPayload.html On my iPhone 13 Pro running iOS 16.1, using breakpoint debugging in Xcode, I don't see the method called when I receive a notification from a Zulip server. However, if I have my dev server send `content-available: 1` in the APNs payload (making it a "background update notification"; see doc linked just above), then I *do* see the method called! I don't know any other way to interpret the qualifier in the method's doc. My experiment was prompted by this Apple forum thread: https://developer.apple.com/forums/thread/128794 It seems like the doc should say more explicitly what kind of notifications will trigger the call. Anyway, we don't currently need to be told at all when a notification is received, whether it's this special kind or not. What we care about is the user's *response* to a notification: when they tap it, we want to bring the user to the relevant conversation in the app. From reading code in @react-native-community/push-notification-ios and from some experiments, this UIApplication method is the only thing that would ever cause our `listenIOS('notification', …)` callback to fire. So, remove that too. …And since we were expecting that JavaScript code to handle the user's notification *response* by navigating to a conversation, we note that that feature must've been broken for a while. Filed just now as issue #5679. It doesn't help that the doc for @react-native-community/push-notification-ios incorrectly identifies 'notification' as the event carrying the user's response: https://github.com/react-native-push-notification/ios#how-to-determine-push-notification-user-click …Anyway, we have a planned fix for #5679, coming next. Thankfully it involves using less of that library, not more. Not marking NFC because, especially with how unhelpful that Apple doc is, I guess it's possible that the system *was* calling our method override even without `content-available: 1` in the APNs payloads. If so, this will be a bugfix: in those cases, before this commit, the app would navigate to a notification's conversation without being prompted by the user tapping on the notification. Searching the tracker, the symptom in issue #4763 matches that description ("[ios] Opening app from recents screen can reopen conversation from previous notification"), but in a quick test just now I wasn't able to reproduce that issue on current `main`, and it's time to move on to other work.
1 parent c62402d commit 4bc7ecc

File tree

2 files changed

+0
-15
lines changed

2 files changed

+0
-15
lines changed

ios/ZulipMobile/AppDelegate.mm

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,6 @@ - (void)application:(UIApplication *)application didFailToRegisterForRemoteNotif
114114
[RNCPushNotificationIOS didFailToRegisterForRemoteNotificationsWithError:error];
115115
}
116116

117-
// Required for the notification event. You must call the completion handler after handling the remote notification.
118-
- (void)application:(UIApplication *)application didReceiveRemoteNotification:(NSDictionary *)userInfo
119-
fetchCompletionHandler:(void (^)(UIBackgroundFetchResult))completionHandler
120-
{
121-
[RNCPushNotificationIOS didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler];
122-
}
123-
124117
// Required for localNotification event
125118
- (void)userNotificationCenter:(UNUserNotificationCenter *)center
126119
didReceiveNotificationResponse:(UNNotificationResponse *)response

src/notification/NotificationListener.js

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import type { GlobalDispatch } from '../types';
88
import { androidGetToken, handleDeviceToken } from './notifTokens';
99
import type { Notification } from './types';
1010
import * as logging from '../utils/logging';
11-
import { fromPushNotificationIOS } from './extract';
1211
import { narrowToNotification } from './notifOpen';
1312

1413
/**
@@ -92,13 +91,6 @@ export default class NotificationListener {
9291
this.listenAndroid('notificationOpened', this.handleNotificationOpen);
9392
this.listenAndroid('remoteNotificationsRegistered', this.handleDeviceToken);
9493
} else {
95-
this.listenIOS('notification', (notification: PushNotificationIOS) => {
96-
const dataFromAPNs = fromPushNotificationIOS(notification);
97-
if (!dataFromAPNs) {
98-
return;
99-
}
100-
this.handleNotificationOpen(dataFromAPNs);
101-
});
10294
this.listenIOS('register', this.handleDeviceToken);
10395
this.listenIOS('registrationError', this.handleIOSRegistrationFailure);
10496
}

0 commit comments

Comments
 (0)