Skip to content

Commit 0d87a06

Browse files
committed
Fix overlapping note_detail() calls orphaning editor instances
Clicking a note re-entered note_detail() while a previous call was still mid-flight, so two overlapping calls both passed the teardown check before either nulled note_split, both created a fresh SplitEditor, and whichever resolved last silently overwrote note_split -- orphaning the loser's instance (websocket, DOM nodes) with no destroy() ever called. Add a per-call token checked after every await so a superseded call detects it lost the race and tears down what it already created. Also defensively remove any stray collab bar/badge left behind by an orphaned instance on create().
1 parent a6a89f2 commit 0d87a06

2 files changed

Lines changed: 46 additions & 5 deletions

File tree

ui/src/pages/case.notes.js

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* Defines the kanban board */
22

33
let note_split;
4+
let note_detail_token = 0;
45
let session_id = null ;
56
let collaborator = null ;
67
let collaborator_socket = null ;
@@ -611,10 +612,26 @@ function wait_for_split_editor() {
611612
}
612613

613614
async function note_detail(id) {
615+
// A click on a note re-enters this function while a previous call is
616+
// still mid-flight (awaiting collab flush, editor creation, ...). Without
617+
// a guard, two overlapping calls both pass the `if (note_split)` teardown
618+
// check before either nulls it out, both create a fresh SplitEditor, and
619+
// whichever resolves last silently overwrites `note_split` -- orphaning
620+
// the loser's instance (its collab websocket, DOM nodes incl. the "Live
621+
// view" badge, which is appended outside the pane container that gets
622+
// cleared on the next create()) with no destroy() ever called on it.
623+
// Bumping this token and checking it after every await lets a superseded
624+
// call detect it lost the race and destroy anything it already created
625+
// instead of publishing it to the shared `note_split` variable.
626+
const token = ++note_detail_token;
614627

615628
get_request_api(`/case/notes/${id}`)
616629
.done(async (data) => {
617630
if (data.status === 'success') {
631+
if (token !== note_detail_token) {
632+
return false;
633+
}
634+
618635
let previous_note_id = $('#currentNoteIDLabel').data('note_id');
619636

620637
if (timer) {
@@ -629,11 +646,20 @@ async function note_detail(id) {
629646
} else if (note_dirty && previous_note_id) {
630647
silent_save_note(previous_note_id, previous_note_markdown);
631648
}
649+
if (token !== note_detail_token) {
650+
// A newer call already owns teardown/creation; don't
651+
// race destroy() on the same instance from two callers.
652+
return false;
653+
}
632654
await note_split.destroy();
633655
note_split = null;
634656
note_dirty = false;
635657
}
636658

659+
if (token !== note_detail_token) {
660+
return false;
661+
}
662+
637663
if (collaborator !== null) {
638664
collaborator.close(previous_note_id);
639665
}
@@ -642,6 +668,9 @@ async function note_detail(id) {
642668
collaborator = null;
643669

644670
await wait_for_split_editor();
671+
if (token !== note_detail_token) {
672+
return false;
673+
}
645674

646675
$('#currentNoteTitle').text(data.data.note_title);
647676
previousNoteTitle = data.data.note_title;
@@ -667,18 +696,19 @@ async function note_detail(id) {
667696
},
668697
},
669698
};
699+
let created_split;
670700
try {
671-
note_split = await window.IrisSplitEditor.create(split_options);
701+
created_split = await window.IrisSplitEditor.create(split_options);
672702
} catch (collab_error) {
673703
delete split_options.collab;
674-
note_split = await window.IrisSplitEditor.create(split_options);
704+
created_split = await window.IrisSplitEditor.create(split_options);
675705
}
676706

677-
if (note_id !== target_note) {
678-
await note_split.destroy();
679-
note_split = null;
707+
if (token !== note_detail_token || note_id !== target_note) {
708+
await created_split.destroy();
680709
return false;
681710
}
711+
note_split = created_split;
682712

683713
note_split.focus();
684714
if (!is_note_collab_active()) {

ui/src/pages/milkdown_split.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,11 @@ class SplitEditor {
359359
return;
360360
}
361361

362+
// Defensive: an orphaned instance (e.g. a superseded note_detail()
363+
// call that never got destroy()-ed) may have left its own bar behind
364+
// in this same parent. Remove stragglers so they can't stack.
365+
parent.querySelectorAll('.iris-collab-bar').forEach((stale) => stale.remove());
366+
362367
this.collabBar = document.createElement('div');
363368
this.collabBar.className = 'iris-collab-bar';
364369

@@ -400,6 +405,12 @@ class SplitEditor {
400405
return;
401406
}
402407

408+
// Defensive: same reasoning as installPresenceBar() above -- an
409+
// orphaned instance's badge lives directly on sourcePane, which is
410+
// never cleared by create() (only the child source/wysiwyg mounts
411+
// are), so it survives re-creation unless removed here.
412+
this.sourcePane.querySelectorAll('.iris-collab-liveview-badge').forEach((stale) => stale.remove());
413+
403414
this.collabSourceBadge = document.createElement('div');
404415
this.collabSourceBadge.className = 'iris-collab-liveview-badge';
405416
this.collabSourceBadge.textContent = 'Live view · edit in WYSIWYG';

0 commit comments

Comments
 (0)