Skip to content

mobile: "jump to oldest unread" silently lands on the newer message when a forced msg: unread is older #4643

Description

@brow

Summary

The mobile "jump to oldest unread" chevron can silently take you to the wrong message. When a channel has both a message-level forced-unread (msg:<id>, e.g. from "Mark as unread" on a single message) and an ordinary unread message, and the forced one is older, the control offers the newer row as "oldest unread". The older forced message is never paged in, so it is never a candidate.

There is no failure signal. The chevron appears, the tap animates, you land somewhere, and the thing you actually marked unread is still further back. This fails in the direction that erodes trust in unread state: the control looks like it worked.

This is pre-existing and was not introduced by #4239 (it reproduces identically at 9dd68e5e5 and a0aaa853b, the two heads before it). Filing it as a tracked follow-up owned by us, not by the #4239 author. It is deliberately not bundled into #4641, which restores a missing chevron: that is a fail-closed bug, this is a silent wrong jump, and they deserve separate review attention.

Where

mobile/lib/features/channels/channel_detail_page/message_list.dart, the unread-boundary effect. The target-selection ternary:

final hasLoadedFetchTarget =
    initialOldestOrdinaryUnreadMessageId != null
    ? hasLoadedOrdinaryTarget
    : hasLoadedForcedTarget;

Whenever an ordinary unread id exists, it becomes the sole fetch target. canFetchTarget is then already satisfied as soon as that ordinary row is loaded, so the walk-back loop never runs and the older forced id never enters entries. Candidate resolution below only considers rows in entries, so the sort over [ordinaryUnread, forcedUnread] picks the ordinary row by default.

The forced-only case works correctly, which is what makes this specifically a mixed-target defect rather than a broken forced-unread feature.

Reproduction (widget-level, executed)

judge1's fixture, run at #4641's head bde10f65cf1a1b7bfa06e2b858273ebf52aaeaa0. Newest page is msg100..msg149, two older pages available (msg50..msg99, msg0..msg49). Read state marks the channel read at 1119 with forcedUnreadContexts: {'msg:msg21': <channel>}. The probe additionally observes ordinary unread msg120; the control does not.

The instrument is the tap outcome, not chevron presence: in a virtualized reversed list, asserting that a control rendered proves nothing about where it goes.

arm fetchOlderCalls chevron after tapping
probe: forced msg21 two pages back + ordinary msg120 loaded 0 present lands msg120, msg21 not rendered
control: forced msg21 only 2 present lands msg21 correctly

The control pages back twice and lands right; the probe does not page at all and lands on the newer row. Same file, same run.

Steps in the product, for a manual repro:

  1. In a channel with more than ~100 messages, scroll well back and "Mark as unread" on a single old message.
  2. Return to the channel and let a newer ordinary message arrive unread (or have one already unread newer than that message).
  3. Reopen the channel and tap the jump-to-oldest-unread chevron. It lands on the newer message, not the one you marked.

No committed test pins the current behavior

mobile/test/features/channels/channel_detail_page_test.dart is indifferent to this in both directions (EXECUTION, at bde10f65):

  • committed file, unmodified: 77 pass / 0 fail
  • with a candidate fix applied (require all known targets loaded, rather than only the ordinary one), plus the two probe cases: 79 pass / 0 fail, and the probe flips to landing msg21 while the control stays correct
  • test/features/channels/ with the candidate fix: 532 pass / 1 skip / 0 fail, unchanged from pristine

So the behavior can be changed in either direction without a single committed assertion noticing. Any fix here should land with a test that pins the destination, not the presence of the chevron.

Why this is not a one-clause fix

Requiring both targets to be loaded changes what gets paged in, which interacts with the four-fetch cap in the same effect: a fix has to say what happens when the older forced target is beyond the cap's reach. The candidate above is a demonstration that the cause is correctly identified, not a proposed patch. Expect it to need its own arms for the cap interaction and for the case where the forced target is unreachable entirely.

Boundary

All numbers above are widget tests driving the real _MessageList and real main-timeline construction through fake message/channel/read-state notifiers, on the pinned Hermit toolchain. No live relay, no real paged window responses, no device rendering, and the deep-link preload path (loadEventsById) is not implemented by the fake, so the production hit rate of this defect is not measured here. Source files were restored and re-hashed to their committed blobs after every mutation; worktree 0-porcelain at bde10f65.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions