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

Abierto
  • ¿Algo no está bien?
#7,792 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
1-2 días
Aptitud para principiantes
30/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
react-native, typescript
Área
frontend, mobile

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

🐛 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).

Lenguaje dominante
TypeScript
Estrellas
2.4k
Forks
1.5k
Merge medio
1 d 15 h
PR fusionados (30 d)
29

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de RocketChat/Rocket.Chat.ReactNative

Todos los issues de RocketChat/Rocket.Chat.ReactNative

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.