diff --git a/e2e/local-homeserver.spec.ts b/e2e/local-homeserver.spec.ts index 7dcbd6f63..bae50395d 100644 --- a/e2e/local-homeserver.spec.ts +++ b/e2e/local-homeserver.spec.ts @@ -265,6 +265,46 @@ test.describe('local homeserver regression', () => { await expect.poll(threadUnread).toBe(false); }); + test("a thread I don't follow stays unread while I view the room (#314)", async ({ page }) => { + const room = await createRoom(alice, 'Unfollowed Thread Room', { invite: [bob.userId] }); + await joinRoom(bob, room); + const inThread = (root: string) => ({ + 'm.relates_to': { + rel_type: 'm.thread', + event_id: root, + is_falling_back: true, + 'm.in_reply_to': { event_id: root }, + }, + }); + const root = await sendText(bob, room, "bob's thread root"); + await loginUI(page, alice); + await openRoom(page, room); + await sendText(bob, room, 'reply in bob thread', inThread(root)); + await sendText(bob, room, 'newer main message 314'); + await expect(page.getByText('newer main message 314')).toBeVisible(); + const threadUnread = async () => { + const filter = { + room: { + rooms: [room], + timeline: { limit: 1, unread_thread_notifications: true, not_types: [uniq('x.')] }, + }, + }; + const sync = await api<{ + rooms: { + join: Record }>; + }; + }>( + 'GET', + `/_matrix/client/v3/sync?timeout=0&filter=${enc(JSON.stringify(filter))}`, + alice.token, + ); + return root in (sync.rooms.join[room]?.unread_thread_notifications ?? {}); + }; + await page.waitForTimeout(2000); // let any receipt the room view would send go out + // Viewing the room used to read every thread you don't follow (#314). + expect(await threadUnread()).toBe(true); + }); + test('timeline image opens the gallery lightbox (#219) @webkit', async ({ page }) => { const room = await createRoom(alice, 'Lightbox Room'); const png = Buffer.from( diff --git a/src/app/utils/notifications.test.ts b/src/app/utils/notifications.test.ts index 36fca6b24..9d8b0a976 100644 --- a/src/app/utils/notifications.test.ts +++ b/src/app/utils/notifications.test.ts @@ -169,7 +169,7 @@ test('private receipt flag uses ReadPrivate', async () => { // follows unread; an explicit mark-as-read still clears everything. const sender = (userId: string) => ({ getSender: () => userId }); -test('passive: a thread I replied in stays unread; main receipt is scoped to main', async () => { +test('passive: threads stay unread, followed or not; main receipt is scoped to main (#314)', async () => { const mine = thread('$mine', evt('$r1'), { hasCurrentUserParticipated: true }); const other = thread('$other', evt('$r2')); const { mx, calls } = setup({ @@ -181,10 +181,9 @@ test('passive: a thread I replied in stays unread; main receipt is scoped to mai await markAsRead(mx, '!r:server', false, { passive: true }); assert.deepEqual( calls.map((c) => [c.eventId, c.unthreaded]), - [ - ['b', false], // main-scoped: unthreaded would also read $mine's reply - ['$r2', false], - ], + // main-scoped (unthreaded would read the threads' replies), and no thread + // receipts: [Gitea #314] unfollowed threads used to be read silently. + [['b', false]], ); }); @@ -203,7 +202,7 @@ test('passive: threads I started or was mentioned in stay unread', async () => { assert.equal(calls.length, 0); }); -test('passive: no followed thread unread → main receipt stays unthreaded', async () => { +test('passive: an unfollowed unread thread is left unread too (#314)', async () => { const other = thread('$other', evt('$r2'), { rootEvent: sender('@bob:server') }); const { mx, calls } = setup({ timeline: [evt('a'), evt('b')], @@ -214,10 +213,16 @@ test('passive: no followed thread unread → main receipt stays unthreaded', asy await markAsRead(mx, '!r:server', false, { passive: true }); assert.deepEqual( calls.map((c) => [c.eventId, c.unthreaded]), - [ - ['b', true], - ['$r2', false], - ], + [['b', false]], + ); +}); + +test('passive: no thread unread → main receipt stays unthreaded', async () => { + const { mx, calls } = setup({ timeline: [evt('a'), evt('b')], readUpTo: 'a', threads: [] }); + await markAsRead(mx, '!r:server', false, { passive: true }); + assert.deepEqual( + calls.map((c) => [c.eventId, c.unthreaded]), + [['b', true]], ); }); @@ -249,9 +254,10 @@ test('passive: the thread open in the panel is left to the panel', async () => { threadUnread: { $open: 1, $other: 1 }, }); await markAsRead(mx, '!r:server', false, { passive: true, openThreadId: '$open' }); + // The open thread is left to the panel, and [#314] the other one stays unread. assert.deepEqual( calls.map((c) => c.eventId), - ['$r2'], + [], ); }); diff --git a/src/app/utils/notifications.ts b/src/app/utils/notifications.ts index 16fd2f4d7..ad3648c7c 100644 --- a/src/app/utils/notifications.ts +++ b/src/app/utils/notifications.ts @@ -1,27 +1,19 @@ -import { MatrixClient, NotificationCountType, ReceiptType, Room, Thread } from 'matrix-js-sdk'; +import { MatrixClient, NotificationCountType, ReceiptType, Thread } from 'matrix-js-sdk'; import { getSettings } from '../state/settings'; import { readMarkedUnread, setMarkedUnread } from '../state/room/markedUnread'; +import { isThreadFollowed } from './room'; -/** - * [Gitea #217] Threads the user is following: they started it, replied in it, - * or were mentioned in it. `hasCurrentUserParticipated` comes from the server's - * thread bundle and lags a reply we just sent, so our own loaded events count too. - */ -export function isThreadFollowed(room: Room, thread: Thread, userId: string | null): boolean { - if (thread.hasCurrentUserParticipated) return true; - if (userId && thread.rootEvent?.getSender() === userId) return true; - if (userId && thread.timeline?.some((e) => e.getSender() === userId)) return true; - return ( - (room.getThreadUnreadNotificationCount(thread.id, NotificationCountType.Highlight) ?? 0) > 0 - ); -} +// isThreadFollowed lives in utils/room (getUnreadInfo needs it too). +export { isThreadFollowed }; export type MarkAsReadOptions = { /** * [Gitea #217] The read comes from simply looking at the room (timeline at - * the bottom and focused), not an explicit "mark as read". Threads the user - * follows stay unread until their panel is opened, like Slack and Discord; - * the rest are cleared so they don't keep the room dot lit forever. + * the bottom and focused), not an explicit "mark as read". Threads stay + * unread until opened (or "Mark all read"), like Slack and Discord. + * [Gitea #314] That includes threads you don't follow; those no longer + * count toward the room's unread dot (see getUnreadInfo), so leaving them + * unread doesn't keep the room lit. */ passive?: boolean; /** @@ -66,7 +58,9 @@ export async function markAsRead( const threads = room.getThreads(); const threadUnread = (thread: Thread) => room.getThreadUnreadNotificationCount(thread.id, NotificationCountType.Total) ?? 0; - const keepThread = (thread: Thread) => passive && isThreadFollowed(room, thread, mx.getUserId()); + // [Gitea #314] A passive read leaves every thread's state alone (it used to + // silently read every thread you don't follow). + const keepThread = (_thread: Thread) => passive; // [Gitea #217] An unthreaded receipt also reads every thread reply older than // it, so a passive read while a followed thread is unread must stay scoped // to the main timeline or the next main message would clear that thread. diff --git a/src/app/utils/room.test.ts b/src/app/utils/room.test.ts index a8973c880..0e06013c2 100644 --- a/src/app/utils/room.test.ts +++ b/src/app/utils/room.test.ts @@ -928,3 +928,42 @@ test('getAllVersionsRoomCreator returns empty set when no create event', () => { test('EventTimeline.FORWARDS constant is available', () => { assert.equal(typeof EventTimeline.FORWARDS, 'string'); }); + +test('getUnreadInfo: unfollowed threads do not light the room; followed and mentioned do (#314)', () => { + const threads = [ + { id: '$followed', hasCurrentUserParticipated: true }, + { id: '$other' }, + { id: '$mention' }, + ]; + const counts: Record = { + $followed: { total: 2, highlight: 0 }, + $other: { total: 3, highlight: 0 }, + $mention: { total: 1, highlight: 1 }, + }; + const room = { + roomId: '!r:x', + // Server room totals include thread counts: 2 + 3 + 1 thread, 0 main. + getUnreadNotificationCount: (type: NotificationCountType) => + type === NotificationCountType.Total ? 6 : 1, + getThreads: () => threads, + getThreadUnreadNotificationCount: (id: string, type: NotificationCountType) => + type === NotificationCountType.Total ? counts[id].total : counts[id].highlight, + getEventReadUpTo: () => null, + getLiveTimeline: () => ({ getEvents: () => [] }), + } as unknown as Room; + const me = { getUserId: () => '@me:x' } as unknown as MatrixClient; + assert.deepEqual(getUnreadInfo(room, undefined, me), { roomId: '!r:x', highlight: 1, total: 3 }); + // Only an unfollowed thread is unread: the room isn't lit. + counts.$followed.total = 0; + counts.$mention = { total: 0, highlight: 0 }; + const onlyOther = { + ...room, + getUnreadNotificationCount: (type: NotificationCountType) => + type === NotificationCountType.Total ? 3 : 0, + } as unknown as Room; + assert.deepEqual(getUnreadInfo(onlyOther, undefined, me), { + roomId: '!r:x', + highlight: 0, + total: 0, + }); +}); diff --git a/src/app/utils/room.ts b/src/app/utils/room.ts index 41071fe52..68a061dc0 100644 --- a/src/app/utils/room.ts +++ b/src/app/utils/room.ts @@ -18,6 +18,7 @@ import { NotificationCountType, RelationType, Room, + Thread, RoomMember, TimelineEvents, IRoomTimelineData, @@ -418,6 +419,20 @@ export const roomHaveUnread = (mx: MatrixClient, room: Room) => { return true; }; +/** + * [Gitea #217] Threads the user is following: they started it, replied in it, + * or were mentioned in it. `hasCurrentUserParticipated` comes from the server's + * thread bundle and lags a reply we just sent, so our own loaded events count too. + */ +export function isThreadFollowed(room: Room, thread: Thread, userId: string | null): boolean { + if (thread.hasCurrentUserParticipated) return true; + if (userId && thread.rootEvent?.getSender() === userId) return true; + if (userId && thread.timeline?.some((e) => e.getSender() === userId)) return true; + return ( + (room.getThreadUnreadNotificationCount(thread.id, NotificationCountType.Highlight) ?? 0) > 0 + ); +} + export const getUnreadInfo = ( room: Room, mutedThreads?: Set, @@ -439,6 +454,18 @@ export const getUnreadInfo = ( if (highlight < 0) highlight = 0; } + // [Gitea #314] Threads you don't follow stay unread in the Threads panel + // (viewing the room no longer reads them) but don't light the room's dot. + // A mention makes a thread followed, so highlights are never subtracted. + if (mx) { + room.getThreads().forEach((thread) => { + if (mutedThreads?.has(thread.id)) return; + if (isThreadFollowed(room, thread, mx.getUserId())) return; + total -= room.getThreadUnreadNotificationCount(thread.id, NotificationCountType.Total) ?? 0; + }); + if (total < 0) total = 0; + } + const resolvedTotal = highlight > total ? highlight : total; // Suppress a spurious Total when the room isn't really unread. Two safe cases,