Skip to content

Bugfix/noid/wrong images shown - #6635

Merged
mahibi merged 2 commits into
masterfrom
bugfix/noid/wrongImagesShown
Sep 3, 2026
Merged

Bugfix/noid/wrong images shown#6635
mahibi merged 2 commits into
masterfrom
bugfix/noid/wrongImagesShown

Conversation

@mahibi

@mahibi mahibi commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Fix wrong image shown in media viewer

Fixes a bug where tapping an image in a chat's grouped image gallery opens the correct image briefly, then a second later flips to a completely different (often much older) image — reproducible
reliably by opening the first image of a group.

Root cause

The media viewer prefetches older conversation history whenever the tapped image sits within the first two positions of the currently-loaded media list (MediaViewerViewModel.loadOlderGroups()).
Once older items are fetched, they're prepended to the pager's item list, which shifts every existing index — the pager then needs to silently jump forward by that shift amount to keep the same
tapped image on screen.

That shift correction was emitted as a one-shot SharedFlow event, collected by a LaunchedEffect that launches once and lives for the screen's lifetime. Its body captured the item count
(items.size) from whatever composition was active when it first launched. By the time the shift event actually fired (after the older-items network fetch completed), the item list had already
grown, but the effect's clamp still used the stale, smaller count — so the pager settled on the wrong, small index instead of the correctly-shifted one, landing on an unrelated older image.

Fix

Moved the shift into MediaViewerViewModel.UiState itself as a PendingShift(id, amount) field, set atomically together with the prepended item list in the same state update. The Compose side now
reads it via LaunchedEffect(uiState.pendingShift?.id), so the item count and the shift always come from the same state snapshot, eliminating the stale-count race.

Also included: a related but separate hardening fix — the on-disk cache used for downloaded chat attachments (shared_attachments/) was keyed only by the sender-controlled, non-unique
file name. Two different attachments sharing a name (e.g. a generically-named pasted screenshot) could resolve to, and overwrite, the same cache file when downloaded concurrently. Cache files
used by the media viewer are now nested under a per-fileId subdirectory (shared_attachments//) so same-named attachments can no longer collide.

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

…sions

Two different attachments sharing a display name (e.g. a generically
named pasted screenshot) resolved to the same file inside the shared
attachment cache directory. Since the media viewer prefetches the
tapped image's neighbors, a colliding neighbor's download could
overwrite the cache file the viewer had just shown, making the open
image flip to a different one moments later. Cache files for the
media viewer are now nested under a per-fileId subdirectory so
same-named attachments can no longer collide.

Assisted-by: Claude Code:claude-sonnet-5

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi mahibi added this to the 25.1.0 milestone Sep 3, 2026
@mahibi mahibi self-assigned this Sep 3, 2026
@mahibi mahibi added the 3. to review Waiting for reviews label Sep 3, 2026
Opening the first image of a group (or any image within the first two
positions of the currently loaded media) triggered an older-history
prefetch that prepends older items to the pager's item list and
shifts the pager to compensate. That shift correction was emitted as
a one-shot SharedFlow event, collected in a long-lived LaunchedEffect
whose closure captured the item count from the composition active
when the effect first launched. By the time the event fired, the item
list had already grown, but the effect's clamp (`items.size` / later
`pagerState.pageCount`) still reflected the stale, smaller count, so
the pager settled on the wrong, small index - showing an unrelated,
often much older image a moment after the tap.

Moved the shift into UiState itself (MediaViewerViewModel.PendingShift)
so the Compose side always applies it from the very same state
snapshot as the already-updated item list, removing the possibility
of a stale item count at the point the correction is applied.

Assisted-by: Claude Code:claude-sonnet-5

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
mahibi force-pushed the bugfix/noid/wrongImagesShown branch from c6101cd to 704697b Compare September 3, 2026 09:10
@mahibi
mahibi merged commit 5166fe7 into master Sep 3, 2026
13 of 18 checks passed
@mahibi
mahibi deleted the bugfix/noid/wrongImagesShown branch September 3, 2026 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant