From 3487a1808d5a4ec8639769baf8b46a264f591044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathana=C3=ABl=20HANNEBERT?= Date: Wed, 2 Sep 2026 18:22:57 +0200 Subject: [PATCH] Fix thread staying unread after a non-counting event follows your reply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `doesTimelineHaveUnreadMessages` judges the timeline against the latest *important* event, which is the newest event from someone else, since our own events never trigger an unread count. Whether that event counts as read then rests on the js-sdk's shortcut, which only fires when we sent the literal last event in the timeline (`userSentLatestEventInThread`). So replying in a thread and then receiving anything that doesn't trigger an unread count — a reaction, an edit, a redaction, a membership change — leaves the thread unread even though we have demonstrably read it. The main timeline is shielded from this by the synthetic read receipt the js-sdk records for our own events; threads have no equivalent, so this is where it surfaces. Treat the timeline as read when our own latest event is newer than the latest important event, rather than requiring it to be the very last one. Fixes https://github.com/element-hq/element-web/issues/34904 --- apps/web/src/Unread.test.ts | 76 ++++++++++++++++++++++++++++++++++++- apps/web/src/Unread.ts | 25 ++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/apps/web/src/Unread.test.ts b/apps/web/src/Unread.test.ts index d187fb41c37..ff7d909dc22 100644 --- a/apps/web/src/Unread.test.ts +++ b/apps/web/src/Unread.test.ts @@ -12,7 +12,7 @@ import { describe, it, expect, beforeEach, beforeAll, vi } from "vitest"; import { MatrixEvent, EventType, MsgType, Room, ReceiptType } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import { makeBeaconEvent, mkEvent, stubClient } from "test-utils"; -import { makeThreadEvents, mkThread, populateThread } from "test-utils/threads"; +import { makeThreadEvent, makeThreadEvents, mkThread, populateThread } from "test-utils/threads"; import { haveRendererForEvent } from "./events/EventTileFactory"; import { @@ -164,6 +164,30 @@ describe("Unread", () => { expect(doesRoomHaveUnreadMessages(room, false)).toBe(false); }); + it("returns true when another user's message arrives after ours", () => { + room.addLiveEvents( + [ + mkEvent({ + event: true, + type: "m.room.message", + user: myId, + room: roomId, + content: {}, + }), + mkEvent({ + event: true, + type: "m.room.message", + user: aliceId, + room: roomId, + content: {}, + }), + ], + { addToState: true }, + ); + + expect(doesRoomHaveUnreadMessages(room, false)).toBe(true); + }); + it("returns false for a room when the read receipt is at the latest event", () => { const receipt = new MatrixEvent({ type: "m.receipt", @@ -575,6 +599,56 @@ describe("Unread", () => { expect(doesRoomHaveUnreadThreads(room)).toBe(true); }); + it("returns false when we replied and a non-counting event landed after our reply", async () => { + // Thread: root(alice) -> reply(me) -> redacted(alice). Our reply is newer than the + // only unread-triggering event, so we have clearly seen the thread, but we are no + // longer the sender of the literal last event. + const { rootEvent, events } = await populateThread({ + room, + client, + authorId: aliceId, + participantUserIds: [myId], + }); + + const trailing = makeThreadEvent({ + event: true, + user: aliceId, + room: roomId, + msg: "redacted", + rootEventId: rootEvent.getId()!, + replyToEventId: events.at(-1)!.getId()!, + ts: 100, + }); + vi.spyOn(trailing, "isRedacted").mockReturnValue(true); + await room.addLiveEvents([trailing], { addToState: false }); + + expect(doesRoomHaveUnreadThreads(room)).toBe(false); + }); + + it("returns true when an incoming message landed after our reply", async () => { + // Thread: root(alice) -> reply(me) -> message(alice). The trailing message does + // trigger an unread count, so the thread is genuinely unread. + const { rootEvent, events } = await populateThread({ + room, + client, + authorId: aliceId, + participantUserIds: [myId], + }); + + const trailing = makeThreadEvent({ + event: true, + user: aliceId, + room: roomId, + msg: "a real reply", + rootEventId: rootEvent.getId()!, + replyToEventId: events.at(-1)!.getId()!, + ts: 100, + }); + await room.addLiveEvents([trailing], { addToState: false }); + + expect(doesRoomHaveUnreadThreads(room)).toBe(true); + }); + it("return false when we have a receipt for the thread", async () => { const { events, rootEvent } = await populateThread({ room, diff --git a/apps/web/src/Unread.ts b/apps/web/src/Unread.ts index d6a80a8f97e..20ff49b2dc0 100644 --- a/apps/web/src/Unread.ts +++ b/apps/web/src/Unread.ts @@ -71,6 +71,12 @@ function doesTimelineHaveUnreadMessages(room: Room, timeline: Array const myUserId = room.client.getSafeUserId(); const latestImportantEventId = findLatestImportantEvent(room.client, timeline)?.getId(); if (latestImportantEventId) { + // Sending a message means we have seen everything before it. The js-sdk applies this + // shortcut only when our event is the *literal* last one in the timeline, so any event + // that doesn't trigger an unread count (a reaction, an edit, a membership change...) + // landing after our message would otherwise make the timeline unread again. + if (userSentEventAfterLatestImportantEvent(room.client, timeline)) return false; + return !room.hasUserReadEvent(myUserId, latestImportantEventId); } else { // We couldn't find an important event to check - check the unimportant ones. @@ -141,6 +147,25 @@ function findLatestImportantEvent(client: MatrixClient, timeline: Array): boolean { + const myUserId = client.getSafeUserId(); + for (let index = timeline.length - 1; index >= 0; index--) { + const event = timeline[index]; + if (isImportantEvent(client, event)) return false; + // Ignore local echoes that haven't made it to the server yet: they carry no timeline + // position, and a failed send shouldn't mark anything as read. + if (event.getSender() === myUserId && !event.status) return true; + } + return false; +} + /** * Given this event does not have a receipt, is it important enough to make * this room unread?