bug: jumping to an old thread reply or the thread parent silently fails in threads with 300+ replies

Aperta
  • Qualcosa non va?
#7,792 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
1-2 giorni
Idoneità per principianti
30/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
react-native, typescript
Ambito
frontend, mobile

Direzione di ricerca

Start in app/views/RoomView/List/hooks/useScroll.ts, where onReObserve polls fetchMessages() up to MAX_JUMP_GROWTH_RETRIES and growsWindowTowardTarget decides when it runs, then read useMessages for the window count and app/lib/methods/loadThreadMessages.ts for loadThreadMessagesUntil. Done means a jump to the earliest reply or the thread parent in a 500+ reply thread scrolls to and highlights the target, while existing jumps keep working. The work depends on the unmerged #7773, so check its state before starting.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

🐛 bug type: bug
Describe the Bug

In a thread with roughly 300 or more replies, jumping to an old reply (quote link, search result, deep link) or to the thread parent does nothing. The loading overlay closes after about 5 seconds, the list does not move, and no error is shown.

This was found while reviewing #7773, which loads threads 50 replies at a time on servers >= 8.8.0. It is listed under "Known limits" in that PR and was raised in this review comment, where the reviewer suggested handling it as a follow-up.

Why it happens

A jump inside a thread has two stages:

  1. Storing the target. loadThreadMessagesUntil (app/lib/methods/loadThreadMessages.ts) pages the thread until the target is stored in thread_messages. This works at any depth.
  2. Showing it. The list only renders what its Message Window holds, so after the target is stored the window still has to grow until it contains the target. useScroll (app/views/RoomView/List/hooks/useScroll.ts) does this by polling in onReObserve: each time the target is not in the list it calls fetchMessages(), which grows the window by QUERY_SIZE (50), and waits for the next emit.

That polling is capped by MAX_JUMP_GROWTH_RETRIES = 5, and each growth step re-arms JUMP_SAFETY_TIMEOUT (5 s). The window therefore reaches about 300 rows (50 initial + 5 x 50). A target deeper than that never appears, the safety timeout fires, and abortJump resolves the jump without a scroll or an error.

The same distance problem exists one layer down: MAX_SCROLL_TO_INDEX_RETRIES was raised from 20 to 60 in #7773 so deep targets can be reached by scrollToIndex.

On develop this growth only applied to the main room's Anchored Window (jump.anchored). #7773 extended it to threads (|| !!tmid) so jumps inside a thread can reach replies outside the first window, and the same cap now limits them. #7773 also stops appending the thread parent below the newest 50 replies, so a jump to the parent now needs the window to reach the oldest reply. Per #7773's notes the limit applies on any server version.

Steps to Reproduce
  1. Use a server >= 8.8.0 and create a thread with about 500 replies.
  2. Open the thread in the app.
  3. Jump to one of the earliest replies (link to the reply from inside the thread, a search result, or a deep link). A link to the thread parent behaves the same way.
  4. All pages are requested (offset 0, 50, ... up to the page holding the target) and the window grows to about 300 rows.
Expected Behavior

The list scrolls to the target and highlights it.

Actual Behavior

After about 5 seconds the overlay closes with no scroll and no error.

Proposed approach

The target's ts is known once loadThreadMessagesUntil has stored it, so the window can be sized in one step instead of polled:

  • Add a growWindowTo(ts) to useMessages. It sets count.current to the number of rows with ts >= targetTs plus QUERY_SIZE, then re-subscribes once. raiseOrReleaseAnchor already does the same kind of sizing with its aboveCount query (fetchCount()).
  • That would remove growsWindowTowardTarget and the tmid parameter of useScroll, and may let the scroll-to-index retry cap go back to 20.

Things to check when picking this up:

  • The count must use the same filters as the window query (rid = tmid, buildVisibleSystemTypesClause(hideSystemMessages)), otherwise the window is sized too small or too large. Over-counting only adds harmless margin; under-counting reproduces the bug.
  • growsWindowTowardTarget is jump.anchored || !!tmid, so the main room's Anchored Window jumps use the same polling. Decide whether that path keeps it or moves to the same approach.
  • A window of several hundred rows has a render and re-emit cost. Worth measuring on a low-end Android device with a 500+ reply thread.
  • Related and separate: on a slow connection the 5 s highlight (HIGHLIGHT_TIMEOUT) can expire before a deep jump arrives. Also listed in #7773's known limits.
Acceptance
  • In a thread of 500+ replies, jumping to the earliest reply and to the thread parent scrolls to and highlights the target on servers >= 8.8.0 and on servers below it.
  • Jumps within the first 300 rows and Anchored Window jumps in the main room behave as before.
  • Unit tests cover the window sizing, and the termination test for the scroll-to-index retries still passes.
Rocket.Chat Server Version

= 8.8.0 (paged thread loading). The window-growth limit itself is server-version independent.

Rocket.Chat App Version

Needs #7773 (not yet merged at the time of writing).

Lingua principale
TypeScript
Stelle
2.4k
Fork
1.5k
Merge medio
1g 15h
PR unite (30g)
29

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di RocketChat/Rocket.Chat.ReactNative

Tutte le issue di RocketChat/Rocket.Chat.ReactNative

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.