diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index a81712756..e86840ec5 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -303,8 +303,13 @@ describe("EventTimelineSet", () => { messageEventIsDecryptionFailureSpy.mockReturnValue(true); replyEventIsDecryptionFailureSpy.mockReturnValue(true); - messageEvent.emit(MatrixEventEvent.Decrypted, messageEvent); - replyEvent.emit(MatrixEventEvent.Decrypted, replyEvent); + messageEvent.emit( + MatrixEventEvent.Decrypted, + messageEvent, + undefined, + messageEvent.getPushDetails(), + ); + replyEvent.emit(MatrixEventEvent.Decrypted, replyEvent, undefined, replyEvent.getPushDetails()); // simulate decryption messageEventIsDecryptionFailureSpy.mockReturnValue(false); @@ -313,8 +318,13 @@ describe("EventTimelineSet", () => { messageEventShouldAttemptDecryptionSpy.mockReturnValue(false); replyEventShouldAttemptDecryptionSpy.mockReturnValue(false); - messageEvent.emit(MatrixEventEvent.Decrypted, messageEvent); - replyEvent.emit(MatrixEventEvent.Decrypted, replyEvent); + messageEvent.emit( + MatrixEventEvent.Decrypted, + messageEvent, + undefined, + messageEvent.getPushDetails(), + ); + replyEvent.emit(MatrixEventEvent.Decrypted, replyEvent, undefined, replyEvent.getPushDetails()); }); itShouldReturnTheRelatedEvents(); diff --git a/spec/unit/notifications.spec.ts b/spec/unit/notifications.spec.ts index b0ebd23d9..2f845f34d 100644 --- a/spec/unit/notifications.spec.ts +++ b/spec/unit/notifications.spec.ts @@ -54,7 +54,7 @@ describe("fixNotificationCountOnDecryption", () => { mockClient = getMockClientWithEventEmitter({ ...mockClientMethodsUser(), isInitialSyncComplete: jest.fn().mockReturnValue(false), - getPushActionsForEvent: jest.fn().mockReturnValue(mkPushAction(true, true)), + getPushActionsForEvent: jest.fn((ev) => event.getPushActions() ?? mkPushAction(true, true)), getRoom: jest.fn().mockImplementation(() => room), decryptEventIfNeeded: jest.fn().mockResolvedValue(void 0), supportsThreads: jest.fn().mockReturnValue(true), @@ -125,15 +125,15 @@ describe("fixNotificationCountOnDecryption", () => { room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total, 1); room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight, 0); - event.getPushActions = jest.fn().mockReturnValue(mkPushAction(false, false)); - threadEvent.getPushActions = jest.fn().mockReturnValue(mkPushAction(false, false)); + event.getPushActions = jest.fn().mockReturnValue(mkPushAction(true, true)); + threadEvent.getPushActions = jest.fn().mockReturnValue(mkPushAction(true, true)); }); it("changes the room count to highlight on decryption", () => { expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(2); expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(0); - fixNotificationCountOnDecryption(mockClient, event); + fixNotificationCountOnDecryption(mockClient, event, {}); expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(3); expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(1); @@ -143,7 +143,7 @@ describe("fixNotificationCountOnDecryption", () => { room.setUnreadNotificationCount(NotificationCountType.Total, 0); room.setUnreadNotificationCount(NotificationCountType.Highlight, 0); - fixNotificationCountOnDecryption(mockClient, event); + fixNotificationCountOnDecryption(mockClient, event, {}); expect(room.getRoomUnreadNotificationCount(NotificationCountType.Total)).toBe(1); expect(room.getRoomUnreadNotificationCount(NotificationCountType.Highlight)).toBe(1); @@ -153,7 +153,7 @@ describe("fixNotificationCountOnDecryption", () => { expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(1); expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(0); - fixNotificationCountOnDecryption(mockClient, threadEvent); + fixNotificationCountOnDecryption(mockClient, threadEvent, {}); expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(2); expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(1); @@ -163,7 +163,7 @@ describe("fixNotificationCountOnDecryption", () => { room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total, 0); room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight, 0); - fixNotificationCountOnDecryption(mockClient, event); + fixNotificationCountOnDecryption(mockClient, event, {}); expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(0); expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(0); @@ -187,7 +187,7 @@ describe("fixNotificationCountOnDecryption", () => { event: true, }); - fixNotificationCountOnDecryption(mockClient, unknownThreadEvent); + fixNotificationCountOnDecryption(mockClient, unknownThreadEvent, {}); expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(0); expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(0); @@ -201,7 +201,7 @@ describe("fixNotificationCountOnDecryption", () => { event.getPushActions = jest.fn().mockReturnValue(mkPushAction(true, false)); mockClient.getPushActionsForEvent = jest.fn().mockReturnValue(mkPushAction(false, false)); - fixNotificationCountOnDecryption(mockClient, event); + fixNotificationCountOnDecryption(mockClient, event, {}); expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(0); expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(0); }); @@ -213,7 +213,7 @@ describe("fixNotificationCountOnDecryption", () => { threadEvent.getPushActions = jest.fn().mockReturnValue(mkPushAction(true, false)); mockClient.getPushActionsForEvent = jest.fn().mockReturnValue(mkPushAction(false, false)); - fixNotificationCountOnDecryption(mockClient, event); + fixNotificationCountOnDecryption(mockClient, event, {}); expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(0); expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(0); }); @@ -231,4 +231,15 @@ describe("fixNotificationCountOnDecryption", () => { room.setThreadUnreadNotificationCount("$123", NotificationCountType.Highlight, 5); expect(cb).toHaveBeenLastCalledWith({ highlight: 5 }, "$123"); }); + + it("should not increment notification count where event was already contributing notification", () => { + expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(2); + expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(0); + + event.getPushActions = jest.fn().mockReturnValue(mkPushAction(true, false)); + fixNotificationCountOnDecryption(mockClient, event, { actions: { notify: true, tweaks: {} } }); + + expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(2); + expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(0); + }); }); diff --git a/spec/unit/room-state.spec.ts b/spec/unit/room-state.spec.ts index c8d249870..261c7b120 100644 --- a/spec/unit/room-state.spec.ts +++ b/spec/unit/room-state.spec.ts @@ -1037,7 +1037,12 @@ describe("RoomState", function () { // this event is a message after decryption decryptingRelatedEvent.event.type = EventType.RoomMessage; - decryptingRelatedEvent.emit(MatrixEventEvent.Decrypted, decryptingRelatedEvent); + decryptingRelatedEvent.emit( + MatrixEventEvent.Decrypted, + decryptingRelatedEvent, + undefined, + decryptingRelatedEvent.getPushDetails(), + ); expect(addLocationsSpy).not.toHaveBeenCalled(); }); diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 5dee47445..df57ea735 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -3428,13 +3428,13 @@ describe("Room", function () { expect(room.polls.get(pollStartEventId)).toBeUndefined(); // now emit a Decrypted event but keep the decryption failure - pollStartEvent.emit(MatrixEventEvent.Decrypted, pollStartEvent); + pollStartEvent.emit(MatrixEventEvent.Decrypted, pollStartEvent, undefined, pollStartEvent.getPushDetails()); // still do not expect a poll to show up for the room expect(room.polls.get(pollStartEventId)).toBeUndefined(); // clear decryption failure and emit a Decrypted event again isDecryptionFailureSpy.mockRestore(); - pollStartEvent.emit(MatrixEventEvent.Decrypted, pollStartEvent); + pollStartEvent.emit(MatrixEventEvent.Decrypted, pollStartEvent, undefined, pollStartEvent.getPushDetails()); // the poll should now show up in the room's polls const poll = room.polls.get(pollStartEventId); diff --git a/src/client.ts b/src/client.ts index f50a1c740..7dc4d211f 100644 --- a/src/client.ts +++ b/src/client.ts @@ -1371,8 +1371,8 @@ export class MatrixClient extends TypedEventEmitter { - fixNotificationCountOnDecryption(this, event); + this.on(MatrixEventEvent.Decrypted, (event, _, pushDetails) => { + fixNotificationCountOnDecryption(this, event, pushDetails); }); // Like above, we have to listen for read receipts from ourselves in order to @@ -9851,26 +9851,23 @@ export class MatrixClient extends TypedEventEmitter 0) { - // TODO: Handle mentions received while the client is offline - // See also https://github.com/vector-im/element-web/issues/9069 - let newCount = currentHighlightCount; - if (newHighlight && !oldHighlight) newCount++; - if (!newHighlight && oldHighlight) newCount--; + // Ensure the unread counts are kept up to date if the event is encrypted + // We also want to make sure that the notification count goes up if we already + // have encrypted events to avoid other code from resetting 'highlight' to zero. + // TODO: Handle mentions received while the client is offline + // See also https://github.com/vector-im/element-web/issues/9069 + for (const type of [NotificationCountType.Highlight, NotificationCountType.Total]) { + let count = room.getUnreadCountForEventContext(type, event); - if (isThreadEvent) { - room.setThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Highlight, newCount); + let oldValue: boolean; + let newValue: boolean; + if (type === NotificationCountType.Total) { + // Total count is used to typically increment a room notification counter, but not loudly highlight it. + // `notify` is used in practice for incrementing the total count + oldValue = !!oldActions?.notify; + newValue = !!actions?.notify; } else { - room.setUnreadNotificationCount(NotificationCountType.Highlight, newCount); + oldValue = !!oldActions?.tweaks?.highlight; + newValue = !!actions?.tweaks?.highlight; } - } - // Total count is used to typically increment a room notification counter, but not loudly highlight it. - const currentTotalCount = room.getUnreadCountForEventContext(NotificationCountType.Total, event); + if (oldValue !== newValue || count > 0) { + if (newValue && !oldValue) count++; + if (!newValue && oldValue) count--; - // `notify` is used in practice for incrementing the total count - const newNotify = !!actions?.notify; - - // The room total count is NEVER incremented by the server for encrypted rooms. We basically ignore - // the server here as it's always going to tell us to increment for encrypted events. - if (newNotify) { - if (isThreadEvent) { - room.setThreadUnreadNotificationCount( - event.threadRootId, - NotificationCountType.Total, - currentTotalCount + 1, - ); - } else { - room.setUnreadNotificationCount(NotificationCountType.Total, currentTotalCount + 1); + if (isThreadEvent) { + room.setThreadUnreadNotificationCount(event.threadRootId, type, count); + } else { + room.setUnreadNotificationCount(type, count); + } } } } diff --git a/src/models/event.ts b/src/models/event.ts index 72c6212db..693fa87c2 100644 --- a/src/models/event.ts +++ b/src/models/event.ts @@ -226,7 +226,7 @@ export type MatrixEventHandlerMap = { * @param event - The matrix event which has been decrypted * @param err - The error that occurred during decryption, or `undefined` if no error occurred. */ - [MatrixEventEvent.Decrypted]: (event: MatrixEvent, err?: Error) => void; + [MatrixEventEvent.Decrypted]: (event: MatrixEvent, err: Error | undefined, pushDetails: PushDetails) => void; [MatrixEventEvent.BeforeRedaction]: (event: MatrixEvent, redactionEvent: MatrixEvent) => void; [MatrixEventEvent.VisibilityChange]: (event: MatrixEvent, visible: boolean) => void; [MatrixEventEvent.LocalEventIdReplaced]: (event: MatrixEvent) => void; @@ -916,6 +916,8 @@ export class MatrixEvent extends TypedEventEmitter