Altinity / Altinity/altinity-sql-browser

anchoredPopover.close() can clobber a newer popover sharing the same dom refKey

Aperta
#603 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

inbox
Lingua principale
TypeScript
Stelle
8
Fork
2
Merge medio
1h 34m
PR unite (30g)
6

Descrizione

anchoredPopover's close() (now in src/ui/popover.ts, createAnchoredPopovers) removes whatever node currently occupies app.dom[refKey] without verifying that node is the one it opened:

close() {
  if (getRef(refKey)) { ... /* removes it unconditionally */ }
}

Repro: open popover A on refKey, open popover B on the same refKey (A's slot is now B), then invoke A's retained close() handle — B gets removed/clobbered even though A never touched it, and the keyboard owner it releases may no longer belong to A either (keyboard release itself is idempotent via the released flag in acquireKeyboardOwner; the DOM removal is not).

This predates the #588 phase-4 composition-root refactor (the code moved verbatim from src/ui/app.ts, byte-identical) — surfaced during that refactor's readiness review, not introduced by it. Deliberately not fixed there, per this repo's "surface out-of-scope findings, don't bury them" discipline: fixing it would have been a behavior change inside a PR whose scope and review budget were both set for a pure structural extraction.

A regression test pinning the current (buggy) behavior lives in tests/unit/popover.test.ts (search for "I-21" / "stale-clobber").

Suggested fix direction: give each open() call a token/generation and have close() no-op if the ref slot's current occupant token doesn't match the token the closer was handed.

Guida per i contributori

Apri la guida per i contributori

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.

Direzione di ricerca

Inizia in src/ui/popover.ts, all'interno di createAnchoredPopovers, e confronta il percorso di chiusura con la regressione I-21/stale-clobber in tests/unit/popover.test.ts. Esegui prima quel test, quindi aggiorna il comportamento in modo che un close handle conservato non possa rimuovere un popover più recente che occupa lo stesso refKey. Il lavoro è completato quando il popover più recente rimane presente mentre quello originale può ancora essere chiuso in sicurezza.

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

Valutazione

Stack tecnologico
typescript
Ambito
frontend
Tipo di issue
Bug
Difficoltà
3/5
Tempo stimato
1-2 giorni
Stato di attività
Tranquilla
Chiarezza
Specificata chiaramente
Idoneità per principianti
74/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.