fix(reader): stop a quick-deleted highlight from being re-drawn (#4773) (#4779)

The per-relocate re-apply effect reads a memoized annotation index. A
highlight deleted in place after the index was built still sits in its
bucket, and selectLocationAnnotations trusted the build-time deletedAt
filter, so the effect re-drew the just-deleted overlay and left it
orphaned on the page until the book was reopened.

Re-check deletedAt at the read site: in selectLocationAnnotations and in
the sibling globals re-apply loop in Annotator.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Huang Xin
2026-06-25 15:33:55 +08:00
committed by GitHub
parent cecb1c5312
commit 0589cb4f4a
3 changed files with 24 additions and 0 deletions
@@ -54,6 +54,22 @@ describe('buildAnnotationIndex / selectLocationAnnotations', () => {
expect(globals).toEqual([]);
});
it('skips an annotation deleted in place after the index was built (stale index)', () => {
// #4773: the re-apply effect holds a memoized index. A quick delete stamps
// `deletedAt` on the SAME object that is still sitting in the bucket (the
// memo has not recomputed yet). If the read trusts the build-time filter,
// the just-deleted highlight gets re-drawn and its overlay is orphaned —
// visible on the page until the book is reopened. The read must re-check.
const highlight = note({ style: 'highlight', color: 'yellow', note: 'hi' });
const index = buildAnnotationIndex([highlight]);
// The user deletes the highlight: `deletedAt` is stamped in place on the
// booknote object that the stale index still references.
highlight.deletedAt = 123;
const { annotations, notes } = selectLocationAnnotations(index, location);
expect(annotations).toEqual([]);
expect(notes).toEqual([]);
});
it('classifies a styled annotation into annotations only', () => {
const highlight = note({ style: 'highlight', color: 'yellow' });
const index = buildAnnotationIndex([highlight]);
@@ -985,6 +985,10 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({
// covered by `onCreateOverlay`. Using the pre-built `globals`
// array avoids re-walking booknotes per page turn.
for (const annotation of annotationIndex.globals) {
// Same stale-index guard as selectLocationAnnotations: a global deleted
// in place after the memoized index was built must not be re-fanned out
// across sections, which would orphan its overlays (#4773).
if (annotation.deletedAt) continue;
if (view) expandAllRenderedSections(view, annotation);
}
} catch (e) {
@@ -70,6 +70,10 @@ export function selectLocationAnnotations(
const candidates = index.bySection.get(sectionKey) ?? [];
const matchesLocation = createCfiLocationMatcher(location);
for (const item of candidates) {
// Re-check `deletedAt` at read time: the index is memoized, so a highlight
// deleted in place after the index was built still sits in this bucket. Re-
// drawing it here would orphan its overlay (visible until reopen) — #4773.
if (item.deletedAt) continue;
if (!matchesLocation(item.cfi)) continue;
if (item.style) annotations.push(item);
if (item.note && item.note.trim().length > 0) notes.push(item);