From b0053f36d3630f91e3f29694366ef87aff8a2b59 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 17:43:24 +0100 Subject: [PATCH 1/4] Fix instances of event.sender being read for just the userId - this field may not be set in time --- src/Notifier.ts | 2 +- src/Unread.ts | 6 ++---- src/components/structures/MessagePanel.tsx | 2 +- src/components/structures/TimelinePanel.tsx | 7 +++---- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Notifier.ts b/src/Notifier.ts index 415adcafc8..1137e44aec 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -328,7 +328,7 @@ export const Notifier = { onEvent: function(ev: MatrixEvent) { if (!this.isSyncing) return; // don't alert for any messages initially - if (ev.sender && ev.sender.userId === MatrixClientPeg.get().credentials.userId) return; + if (ev.getSender() === MatrixClientPeg.get().credentials.userId) return; MatrixClientPeg.get().decryptEventIfNeeded(ev); diff --git a/src/Unread.ts b/src/Unread.ts index 72f0bb4642..da5b883f92 100644 --- a/src/Unread.ts +++ b/src/Unread.ts @@ -30,7 +30,7 @@ import { haveTileForEvent } from "./components/views/rooms/EventTile"; * @returns {boolean} True if the given event should affect the unread message count */ export function eventTriggersUnreadCount(ev: MatrixEvent): boolean { - if (ev.sender && ev.sender.userId == MatrixClientPeg.get().credentials.userId) { + if (ev.getSender() === MatrixClientPeg.get().credentials.userId) { return false; } @@ -63,9 +63,7 @@ export function doesRoomHaveUnreadMessages(room: Room): boolean { // https://github.com/vector-im/element-web/issues/2427 // ...and possibly some of the others at // https://github.com/vector-im/element-web/issues/3363 - if (room.timeline.length && - room.timeline[room.timeline.length - 1].sender && - room.timeline[room.timeline.length - 1].sender.userId === myUserId) { + if (room.timeline.length && room.timeline[room.timeline.length - 1].getSender() === myUserId) { return false; } diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index a0a1ac9b10..47f8c218dc 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -401,7 +401,7 @@ export default class MessagePanel extends React.Component { // TODO: Implement granular (per-room) hide options public shouldShowEvent(mxEv: MatrixEvent): boolean { - if (mxEv.sender && MatrixClientPeg.get().isUserIgnored(mxEv.sender.userId)) { + if (MatrixClientPeg.get().isUserIgnored(mxEv.getSender())) { return false; // ignored = no show (only happens if the ignore happens after an event was received) } diff --git a/src/components/structures/TimelinePanel.tsx b/src/components/structures/TimelinePanel.tsx index c21aac790b..5f9d9b7026 100644 --- a/src/components/structures/TimelinePanel.tsx +++ b/src/components/structures/TimelinePanel.tsx @@ -555,9 +555,8 @@ class TimelinePanel extends React.Component { // more than the timeout on userActiveRecently. // const myUserId = MatrixClientPeg.get().credentials.userId; - const sender = ev.sender ? ev.sender.userId : null; callRMUpdated = false; - if (sender != myUserId && !UserActivity.sharedInstance().userActiveRecently()) { + if (ev.getSender() !== myUserId && !UserActivity.sharedInstance().userActiveRecently()) { updatedState.readMarkerVisible = true; } else if (lastLiveEvent && this.getReadMarkerPosition() === 0) { // we know we're stuckAtBottom, so we can advance the RM @@ -863,7 +862,7 @@ class TimelinePanel extends React.Component { const myUserId = MatrixClientPeg.get().credentials.userId; for (i++; i < events.length; i++) { const ev = events[i]; - if (!ev.sender || ev.sender.userId != myUserId) { + if (ev.getSender() !== myUserId) { break; } } @@ -1337,7 +1336,7 @@ class TimelinePanel extends React.Component { } const shouldIgnore = !!ev.status || // local echo - (ignoreOwn && ev.sender && ev.sender.userId == myUserId); // own message + (ignoreOwn && ev.getSender() === myUserId); // own message const isWithoutTile = !haveTileForEvent(ev) || shouldHideEvent(ev, this.context); if (isWithoutTile || !node) { From 923d68a0fa8f014f28a31b0156ccd0b00e08ed90 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 17:46:46 +0100 Subject: [PATCH 2/4] Fix EventIndex handling events twice It awaits the decryption in onRoomTimeline as well as subscribing to EVent.decrypted --- src/indexing/EventIndex.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/indexing/EventIndex.ts b/src/indexing/EventIndex.ts index a5827fc599..a7142010f2 100644 --- a/src/indexing/EventIndex.ts +++ b/src/indexing/EventIndex.ts @@ -67,7 +67,6 @@ export default class EventIndex extends EventEmitter { client.on('sync', this.onSync); client.on('Room.timeline', this.onRoomTimeline); - client.on('Event.decrypted', this.onEventDecrypted); client.on('Room.timelineReset', this.onTimelineReset); client.on('Room.redaction', this.onRedaction); client.on('RoomState.events', this.onRoomStateEvent); @@ -82,7 +81,6 @@ export default class EventIndex extends EventEmitter { client.removeListener('sync', this.onSync); client.removeListener('Room.timeline', this.onRoomTimeline); - client.removeListener('Event.decrypted', this.onEventDecrypted); client.removeListener('Room.timelineReset', this.onTimelineReset); client.removeListener('Room.redaction', this.onRedaction); client.removeListener('RoomState.events', this.onRoomStateEvent); @@ -221,18 +219,6 @@ export default class EventIndex extends EventEmitter { } }; - /* - * The Event.decrypted listener. - * - * Checks if the event was marked for addition in the Room.timeline - * listener, if so queues it up to be added to the index. - */ - private onEventDecrypted = async (ev: MatrixEvent, err: Error) => { - // If the event isn't in our live event set, ignore it. - if (err) return; - await this.addLiveEventToIndex(ev); - }; - /* * The Room.redaction listener. * From 14371882828acb8aa7abedc76e87715a49563a8e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 18:02:02 +0100 Subject: [PATCH 3/4] Also move effects handling from `event` to `Room.timeline` to wake up less --- src/components/structures/RoomView.tsx | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index 2fe694a435..7e3bcbc962 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -253,7 +253,6 @@ export default class RoomView extends React.Component { this.context.on("userTrustStatusChanged", this.onUserVerificationChanged); this.context.on("crossSigning.keysChanged", this.onCrossSigningKeysChanged); this.context.on("Event.decrypted", this.onEventDecrypted); - this.context.on("event", this.onEvent); // Start listening for RoomViewStore updates this.roomStoreToken = RoomViewStore.addListener(this.onRoomViewStoreUpdate); this.rightPanelStoreToken = RightPanelStore.getSharedInstance().addListener(this.onRightPanelStoreUpdate); @@ -637,7 +636,6 @@ export default class RoomView extends React.Component { this.context.removeListener("userTrustStatusChanged", this.onUserVerificationChanged); this.context.removeListener("crossSigning.keysChanged", this.onCrossSigningKeysChanged); this.context.removeListener("Event.decrypted", this.onEventDecrypted); - this.context.removeListener("event", this.onEvent); } window.removeEventListener('beforeunload', this.onPageUnload); @@ -837,8 +835,7 @@ export default class RoomView extends React.Component { if (this.unmounted) return; // ignore events for other rooms - if (!room) return; - if (!this.state.room || room.roomId != this.state.room.roomId) return; + if (!room || room.roomId !== this.state.room?.roomId) return; // ignore events from filtered timelines if (data.timeline.getTimelineSet() !== room.getUnfilteredTimelineSet()) return; @@ -859,6 +856,10 @@ export default class RoomView extends React.Component { // we'll only be showing a spinner. if (this.state.joining) return; + if (!ev.isBeingDecrypted() && !ev.isDecryptionFailure()) { + this.handleEffects(ev); + } + if (ev.getSender() !== this.context.credentials.userId) { // update unread count when scrolled up if (!this.state.searchResults && this.state.atEndOfLiveTimeline) { @@ -871,20 +872,14 @@ export default class RoomView extends React.Component { } }; - private onEventDecrypted = (ev) => { + private onEventDecrypted = (ev: MatrixEvent) => { + if (!this.state.room || !this.state.matrixClientIsReady) return; // not ready at all + if (ev.getRoomId() !== this.state.room.roomId) return; // not for us if (ev.isDecryptionFailure()) return; this.handleEffects(ev); }; - private onEvent = (ev) => { - if (ev.isBeingDecrypted() || ev.isDecryptionFailure()) return; - this.handleEffects(ev); - }; - - private handleEffects = (ev) => { - if (!this.state.room || !this.state.matrixClientIsReady) return; // not ready at all - if (ev.getRoomId() !== this.state.room.roomId) return; // not for us - + private handleEffects = (ev: MatrixEvent) => { const notifState = RoomNotificationStateStore.instance.getRoomState(this.state.room); if (!notifState.isUnread) return; From 831c4823715f2bfae710d6a652417967eb9ad99f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 18:17:07 +0100 Subject: [PATCH 4/4] Stub out MatrixClient::isUserIgnored for tests --- test/test-utils.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test-utils.js b/test/test-utils.js index ad56522965..d75abc80f0 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -96,6 +96,7 @@ export function createTestClient() { }, }, decryptEventIfNeeded: () => Promise.resolve(), + isUserIgnored: jest.fn().mockReturnValue(false), }; }