diff --git a/e2e/local-homeserver.spec.ts b/e2e/local-homeserver.spec.ts index 78f02e23d..62e41769e 100644 --- a/e2e/local-homeserver.spec.ts +++ b/e2e/local-homeserver.spec.ts @@ -169,6 +169,56 @@ test.describe('local homeserver regression', () => { await ctx.close(); }); + test('a thread I started keeps its unread replies while I view the room (#217)', async ({ + page, + }) => { + const room = await createRoom(alice, 'Thread Unread 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(alice, room, 'my thread root'); + await loginUI(page, alice); + await openRoom(page, room); + // While the room is open and at the bottom: a thread reply, then a newer + // main-timeline message. Neither may clear the thread without opening it. + await sendText(bob, room, 'reply in your thread', inThread(root)); + await sendText(bob, room, 'newer main message'); + await expect(page.getByText('newer main message')).toBeVisible(); + const chip = page + .locator('[data-message-item]', { hasText: 'my thread root' }) + .getByRole('button', { name: /1 reply/ }); + const threadUnread = async () => { + // not_types varies so Synapse's sync response cache can't serve a stale answer. + 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 + expect(await threadUnread()).toBe(true); + await expect(chip).toHaveAccessibleName(/unread replies/); + await chip.click(); + await expect.poll(threadUnread).toBe(false); + }); + 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/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index e42b96731..ab0c76973 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -108,7 +108,7 @@ import { getIntersectionObserverEntry, useIntersectionObserver, } from '../../hooks/useIntersectionObserver'; -import { markAsRead } from '../../utils/notifications'; +import { markAsRead, MarkAsReadOptions } from '../../utils/notifications'; import { useDebounce } from '../../hooks/useDebounce'; import { getResizeObserverEntry, useResizeObserver } from '../../hooks/useResizeObserver'; import * as css from './RoomTimeline.css'; @@ -505,6 +505,11 @@ export function RoomTimeline({ room, eventId, roomInputRef, editor }: RoomTimeli const setReplyDraft = useSetAtom(roomIdToReplyDraftAtomFamily(room.roomId)); const setActiveThreadId = useSetAtom(roomIdToActiveThreadIdAtomFamily(room.roomId)); + // [Gitea #217] Reads from just viewing the timeline are passive: threads the + // user follows stay unread, and the open thread panel sends its own receipts. + const activeThreadId = useAtomValue(roomIdToActiveThreadIdAtomFamily(room.roomId)); + const passiveReadRef = useRef({ passive: true }); + passiveReadRef.current = { passive: true, openThreadId: activeThreadId ?? undefined }; // Thread summary chips only mount for events that already carry thread data // (perf: a chip subscribes room-level listeners, so mounting one per rendered // message would exceed the SDK's emitter cap). This single room-level @@ -707,7 +712,10 @@ export function RoomTimeline({ room, eventId, roomInputRef, editor }: RoomTimeli // and either there are no unread messages or the latest message is from the current user. // If either condition is met, trigger the markAsRead function to send a read receipt. const _roomId = mEvt.getRoomId(); - if (_roomId) requestAnimationFrame(() => markAsRead(mx, _roomId, hideActivity)); + if (_roomId) + requestAnimationFrame(() => + markAsRead(mx, _roomId, hideActivity, passiveReadRef.current), + ); } if (!document.hasFocus() && !unreadInfo) { @@ -829,13 +837,17 @@ export function RoomTimeline({ room, eventId, roomInputRef, editor }: RoomTimeli const tryAutoMarkAsRead = useCallback(() => { const readUptoEventId = readUptoEventIdRef.current; if (!readUptoEventId) { - requestAnimationFrame(() => markAsRead(mx, room.roomId, hideActivity)); + requestAnimationFrame(() => + markAsRead(mx, room.roomId, hideActivity, passiveReadRef.current), + ); return; } const evtTimeline = getEventTimeline(room, readUptoEventId); const latestTimeline = evtTimeline && getFirstLinkedTimeline(evtTimeline, Direction.Forward); if (latestTimeline === room.getLiveTimeline()) { - requestAnimationFrame(() => markAsRead(mx, room.roomId, hideActivity)); + requestAnimationFrame(() => + markAsRead(mx, room.roomId, hideActivity, passiveReadRef.current), + ); } }, [mx, room, hideActivity]); diff --git a/src/app/utils/notifications.test.ts b/src/app/utils/notifications.test.ts index 3f4f4bb4b..36fca6b24 100644 --- a/src/app/utils/notifications.test.ts +++ b/src/app/utils/notifications.test.ts @@ -13,13 +13,16 @@ type ReceiptCall = { eventId: string; receiptType: ReceiptType; unthreaded?: boo const evt = (id: string, sending = false) => ({ getId: () => id, isSending: () => sending }) as any; -const thread = (id: string, lastReply: any) => ({ id, lastReply: () => lastReply }) as any; +const thread = (id: string, lastReply: any, extra: Record = {}) => + ({ id, lastReply: () => lastReply, ...extra }) as any; type RoomOpts = { timeline?: any[]; readUpTo?: string | null; threads?: any[]; threadUnread?: Record; + threadHighlight?: Record; + readEvents?: string[]; markedUnread?: boolean; }; @@ -30,8 +33,12 @@ const setup = (opts: RoomOpts) => { getLiveTimeline: () => ({ getEvents: () => opts.timeline ?? [] }), getEventReadUpTo: () => opts.readUpTo ?? null, getThreads: () => opts.threads ?? [], - getThreadUnreadNotificationCount: (threadId: string, _type: NotificationCountType) => - opts.threadUnread?.[threadId] ?? 0, + hasUserReadEvent: (_userId: string, eventId: string) => + (opts.readEvents ?? []).includes(eventId), + getThreadUnreadNotificationCount: (threadId: string, type: NotificationCountType) => + (type === NotificationCountType.Highlight ? opts.threadHighlight : opts.threadUnread)?.[ + threadId + ] ?? 0, getAccountData: (type: string) => opts.markedUnread && type === 'm.marked_unread' ? { getContent: () => ({ unread: true }) } @@ -157,3 +164,127 @@ test('private receipt flag uses ReadPrivate', async () => { await markAsRead(mx, '!r:server', true); assert.equal(calls[0].receiptType, ReceiptType.ReadPrivate); }); + +// [Gitea #217] A passive read (just looking at the room) keeps threads the user +// 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 () => { + const mine = thread('$mine', evt('$r1'), { hasCurrentUserParticipated: true }); + const other = thread('$other', evt('$r2')); + const { mx, calls } = setup({ + timeline: [evt('a'), evt('b')], + readUpTo: 'a', + threads: [mine, other], + threadUnread: { $mine: 1, $other: 1 }, + }); + 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], + ], + ); +}); + +test('passive: threads I started or was mentioned in stay unread', async () => { + const started = thread('$started', evt('$r1'), { rootEvent: sender('@me:server') }); + const byTimeline = thread('$t', evt('$r2'), { timeline: [sender('@me:server')] }); + const mentioned = thread('$mention', evt('$r3')); + const { mx, calls } = setup({ + timeline: [evt('a')], + readUpTo: 'a', + threads: [started, byTimeline, mentioned], + threadUnread: { $started: 1, $t: 1, $mention: 1 }, + threadHighlight: { $mention: 1 }, + }); + await markAsRead(mx, '!r:server', false, { passive: true }); + assert.equal(calls.length, 0); +}); + +test('passive: no followed thread unread → main receipt stays unthreaded', async () => { + const other = thread('$other', evt('$r2'), { rootEvent: sender('@bob:server') }); + const { mx, calls } = setup({ + timeline: [evt('a'), evt('b')], + readUpTo: 'a', + threads: [other], + threadUnread: { $other: 1 }, + }); + await markAsRead(mx, '!r:server', false, { passive: true }); + assert.deepEqual( + calls.map((c) => [c.eventId, c.unthreaded]), + [ + ['b', true], + ['$r2', false], + ], + ); +}); + +test('explicit mark as read clears followed threads too', async () => { + const mine = thread('$mine', evt('$r1'), { hasCurrentUserParticipated: true }); + const { mx, calls } = setup({ + timeline: [evt('a'), evt('b')], + readUpTo: 'a', + threads: [mine], + threadUnread: { $mine: 1 }, + }); + await markAsRead(mx, '!r:server', false); + assert.deepEqual( + calls.map((c) => [c.eventId, c.unthreaded]), + [ + ['b', true], + ['$r1', false], + ], + ); +}); + +test('passive: the thread open in the panel is left to the panel', async () => { + const open = thread('$open', evt('$r1')); + const other = thread('$other', evt('$r2')); + const { mx, calls } = setup({ + timeline: [evt('a')], + readUpTo: 'a', + threads: [open, other], + threadUnread: { $open: 1, $other: 1 }, + }); + await markAsRead(mx, '!r:server', false, { passive: true, openThreadId: '$open' }); + assert.deepEqual( + calls.map((c) => c.eventId), + ['$r2'], + ); +}); + +test('passive: a followed thread whose count lags is still protected', async () => { + // The reply and a newer main message arrived in the same sync; the thread's + // count is still 0 but its latest reply (from bob) is unread. + const reply = { ...evt('$r1'), getSender: () => '@bob:server' }; + const mine = thread('$mine', reply, { hasCurrentUserParticipated: true }); + const { mx, calls } = setup({ + timeline: [evt('a'), evt('b')], + readUpTo: 'a', + threads: [mine], + threadUnread: { $mine: 0 }, + }); + await markAsRead(mx, '!r:server', false, { passive: true }); + assert.deepEqual( + calls.map((c) => [c.eventId, c.unthreaded]), + [['b', false]], + ); +}); + +test('passive: followed thread already read → main receipt stays unthreaded', async () => { + const reply = { ...evt('$r1'), getSender: () => '@bob:server' }; + const mine = thread('$mine', reply, { hasCurrentUserParticipated: true }); + const { mx, calls } = setup({ + timeline: [evt('a'), evt('b')], + readUpTo: 'a', + threads: [mine], + readEvents: ['$r1'], + }); + await markAsRead(mx, '!r:server', false, { passive: true }); + assert.deepEqual( + calls.map((c) => [c.eventId, c.unthreaded]), + [['b', true]], + ); +}); diff --git a/src/app/utils/notifications.ts b/src/app/utils/notifications.ts index 1930e3f89..16fd2f4d7 100644 --- a/src/app/utils/notifications.ts +++ b/src/app/utils/notifications.ts @@ -1,8 +1,42 @@ -import { MatrixClient, NotificationCountType, ReceiptType } from 'matrix-js-sdk'; +import { MatrixClient, NotificationCountType, ReceiptType, Room, Thread } from 'matrix-js-sdk'; import { getSettings } from '../state/settings'; import { readMarkedUnread, setMarkedUnread } from '../state/room/markedUnread'; -export async function markAsRead(mx: MatrixClient, roomId: string, privateReceipt: boolean) { +/** + * [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 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. + */ + passive?: boolean; + /** + * The thread open in the thread panel, which sends its own receipts; skip it + * here so each reply isn't receipted twice. + */ + openThreadId?: string; +}; + +export async function markAsRead( + mx: MatrixClient, + roomId: string, + privateReceipt: boolean, + { passive = false, openThreadId }: MarkAsReadOptions = {}, +) { const { privateReadReceipts } = getSettings(); const room = mx.getRoom(roomId); if (!room) return; @@ -29,12 +63,32 @@ export async function markAsRead(mx: MatrixClient, roomId: string, privateReceip return null; }; + 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 #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. + // The count can lag: a reply and a newer main message often arrive in the + // same sync, before the thread's count is updated, so also ask whether the + // latest reply has been read. + const myUserId = mx.getUserId(); + const hasUnreadReply = (thread: Thread) => { + if (threadUnread(thread) > 0) return true; + const last = thread.lastReply(); + const lastId = last?.getId(); + if (!last || !lastId || !myUserId || last.getSender() === myUserId) return false; + return !room.hasUserReadEvent(myUserId, lastId); + }; + const keepAnyThread = threads.some((thread) => keepThread(thread) && hasUnreadReply(thread)); + const latestEvent = timeline.length > 0 ? getLatestValidEvent() : null; if (latestEvent) { // Unthreaded receipt: with client threadSupport enabled the SDK would // otherwise scope this to the main timeline (thread_id: "main"). Unthreaded // clears the main timeline + every event up to this one. - await mx.sendReadReceipt(latestEvent, receiptType, true); + await mx.sendReadReceipt(latestEvent, receiptType, !keepAnyThread); } // Clear per-thread notification counts too — the room's unread dot sums them, @@ -50,12 +104,10 @@ export async function markAsRead(mx: MatrixClient, roomId: string, privateReceip // returns null -> the room is reported unread on every mark-read call (this was // the P6 regression, amplified by the bulk mark-all-orphan-rooms-read callers). // If a thread's replies aren't loaded (lastReply() null), just skip it. - const threads = room.getThreads(); await Promise.all( threads.map((thread) => { - const unread = - room.getThreadUnreadNotificationCount(thread.id, NotificationCountType.Total) ?? 0; - if (unread <= 0) return undefined; + if (threadUnread(thread) <= 0) return undefined; + if (keepThread(thread) || thread.id === openThreadId) return undefined; const lastReply = thread.lastReply(); if (!lastReply || lastReply.isSending()) return undefined; // Threaded receipt (unthreaded = false → the SDK scopes it to this thread