Skip to content
Merged
5 changes: 5 additions & 0 deletions .changeset/hidden-room-history-drain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Fixed the message list silently loading the entire room history — and downloading its attachments — in the background when a full-width contextual bar (such as the thread view on small screens) hides it
Comment thread
KevLehman marked this conversation as resolved.
Outdated
38 changes: 38 additions & 0 deletions apps/meteor/client/views/room/body/hooks/useGetMore.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,44 @@ describe('useGetMore', () => {
});
});

it('should not call getMore when the container has no size (hidden behind the contextual bar)', async () => {
const root = mockAppRoot();

const Test = () => {
const [atBottom] = useState(false);
const { innerRef } = useGetMore('room-id', atBottom);
return (
<div style={{ display: 'none' }}>
<div ref={innerRef as any} style={{ height: '100px', overflowY: 'scroll' }} data-testid='scrollable-element'>
<div style={{ height: '800px' }}></div>
</div>
</div>
);
};
(RoomHistoryManager.isLoading as jest.Mock).mockReturnValue(false);
(RoomHistoryManager.hasMore as jest.Mock).mockReturnValue(true);
(RoomHistoryManager.hasMoreNext as jest.Mock).mockReturnValue(false);
(RoomHistoryManager.getMore as jest.Mock).mockClear();

(getBoundingClientRect as jest.Mock).mockReturnValue({
scrollTop: 0,
clientHeight: 0,
scrollHeight: 0,
});

render(<Test />, {
wrapper: root.build(),
});

const scrollableElement = screen.getByTestId('scrollable-element');
scrollableElement.dispatchEvent(new Event('wheel'));
scrollableElement.dispatchEvent(new Event('scroll'));

await new Promise((resolve) => setTimeout(resolve, 150));

expect(RoomHistoryManager.getMore).not.toHaveBeenCalled();
});

it('should call getMoreNext when scrolling near bottom and hasMoreNext is true', () => {
const root = mockAppRoot();
(RoomHistoryManager.isLoading as jest.Mock).mockReturnValue(false);
Expand Down
4 changes: 4 additions & 0 deletions apps/meteor/client/views/room/body/hooks/useGetMore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ export const useGetMore = (rid: string, isJumpingToMessage: boolean) => {

const { scrollTop, clientHeight, scrollHeight } = getBoundingClientRect(element);

if (clientHeight === 0) {
return;
}

const lastScrollTopRef = scrollTop;
const height = clientHeight;
const hasMore = RoomHistoryManager.hasMore(rid);
Expand Down
56 changes: 56 additions & 0 deletions apps/meteor/tests/e2e/threads.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { faker } from '@faker-js/faker';

import { Users } from './fixtures/userStates';
import { HomeChannel } from './page-objects';
import { createTargetChannel, deleteChannel } from './utils';
Expand Down Expand Up @@ -205,3 +207,57 @@ test.describe.serial('Threads', () => {
});
});
});

test.describe.serial('Threads - small screens', () => {
let poHomeChannel: HomeChannel;
let targetChannel: { name: string; _id: string };

test.beforeAll(async ({ api }) => {
targetChannel = await api
.post('/channels.create', { name: faker.string.uuid() })
.then((res) => res.json())
.then((data) => data.channel);

for (let i = 0; i < 60; i++) {
await api.post('/chat.postMessage', { roomId: targetChannel._id, text: `history message ${i}` });
Comment thread
KevLehman marked this conversation as resolved.
Outdated
}
const { message: parent } = await api
.post('/chat.postMessage', { roomId: targetChannel._id, text: 'thread parent' })
.then((res) => res.json());
await api.post('/chat.postMessage', { roomId: targetChannel._id, text: 'thread reply', tmid: parent._id });
// Without this the room opens at the first unread message and loads the whole history at
// once, leaving no older page for the hidden list to drain.
await api.post('/subscriptions.read', { rid: targetChannel._id });
});

test.afterAll(async ({ api }) => deleteChannel(api, targetChannel.name));

test('should not load older messages while the message list is hidden behind the full-width thread view', async ({ page }) => {
poHomeChannel = new HomeChannel(page);
await poHomeChannel.gotoChannel(targetChannel.name);
// Below the "sm" breakpoint (600px) the contextual bar takes the full room width and the
// message list is hidden behind it while staying mounted.
await page.setViewportSize({ width: 599, height: 700 });

await poHomeChannel.content.mainMessageListScroller.hover();
await page.mouse.wheel(0, -100);

// Role-based locators cannot see the list once it is display:none — count DOM nodes instead
// (thread messages carry a different aria-roledescription, so they never match).
const mainMessageListItems = page.locator('[role="listitem"][aria-roledescription="message"]');
const loadedMessages = await mainMessageListItems.count();
Comment thread
KevLehman marked this conversation as resolved.
// 61 user messages were seeded — an unloaded older page must remain or there is nothing to drain
expect(loadedMessages).toBeLessThan(61);

await poHomeChannel.content.lastUserMessage.getByRole('button', { name: 'View thread' }).click();
await expect(page).toHaveURL(/.*thread/);
await expect(poHomeChannel.content.lastUserThreadMessage).toContainText('thread reply');
await expect(poHomeChannel.content.mainMessageListScroller).toBeHidden();

// A hidden-history drain re-triggers ~every 100ms, growing the list by a 50-message page each
// time — a fixed window is enough to catch it while a stable count proves the guard held.
await page.waitForTimeout(2500);
Comment thread
KevLehman marked this conversation as resolved.
Outdated

expect(await mainMessageListItems.count()).toBe(loadedMessages);
});
});
Loading