From 8eee0f0c9d2830f5c0515b784b8a72ec3ccea34b Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Sun, 16 Aug 2026 17:04:29 -0700 Subject: [PATCH 1/3] support trashing of annotations - if annotation trashing is supported, do not override key of annotations when deletion is undone - expose annotationManager.clearInterferingHistory so that when an annotation is erased from trash, it is removed from reader's edits history --- src/common/annotation-manager.js | 19 +++++++++++++++---- src/common/reader.js | 9 +++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/common/annotation-manager.js b/src/common/annotation-manager.js index d76c9205..c6785a6b 100644 --- a/src/common/annotation-manager.js +++ b/src/common/annotation-manager.js @@ -23,6 +23,7 @@ class AnnotationManager { this._onSave = options.onSave; this._onDelete = options.onDelete; this._onChangeHistory = options.onChangeHistory; + this._trashesAnnotations = options.trashesAnnotations; this._adjustTextAnnotationPosition = options.adjustTextAnnotationPosition; this.render = () => { options.onRender([...this._annotations]); @@ -601,8 +602,10 @@ class AnnotationManager { if (annotation) { annotation.dateModified = (new Date()).toISOString(); } - // Assign new id when undeleting to reduce sync conflicts - if (!prevAnnotation) { + // Assign new id when undeleting to reduce sync conflicts. Clients that + // trash annotations keep the deleted one around, so undeleting restores + // it in place and the id has to stay the same for the client to find it. + if (!prevAnnotation && !this._trashesAnnotations) { let newID = this._generateObjectKey(); mapping.set(annotation.id, newID); annotation.id = newID; @@ -635,8 +638,10 @@ class AnnotationManager { if (annotation) { annotation.dateModified = (new Date()).toISOString(); } - // Assign new id when undeleting to reduce sync conflicts - if (!prevAnnotation) { + // Assign new id when undeleting to reduce sync conflicts. Clients that + // trash annotations keep the deleted one around, so undeleting restores + // it in place and the id has to stay the same for the client to find it. + if (!prevAnnotation && !this._trashesAnnotations) { let newID = this._generateObjectKey(); mapping.set(annotation.id, newID); annotation.id = newID; @@ -654,6 +659,12 @@ class AnnotationManager { return true; } + // Drops history points for annotations that have left the reader so + // that erased annotations cannot be brought back. + clearHistoryForAnnotations(ids) { + this._clearInterferingHistory(ids); + } + _clearInterferingHistory(affectedAnnotationIDs) { for (let i = this._undoStack.length - 1; i >= 0; i--) { if (affectedAnnotationIDs.some(id => this._undoStack[i].annotations.has(id))) { diff --git a/src/common/reader.js b/src/common/reader.js index 71583fdd..b1375f10 100644 --- a/src/common/reader.js +++ b/src/common/reader.js @@ -130,6 +130,7 @@ class Reader { this._onSetPopupPosition = options.onSetPopupPosition; this._onChangeUndoHistory = options.onChangeUndoHistory; this._externalUndoHistory = !!options.onChangeUndoHistory; + this._trashesAnnotations = !!options.trashesAnnotations; for (let ftl of options.ftl) { addFTL(ftl); @@ -361,6 +362,7 @@ class Reader { this._updateState({ filter }); }, onChangeHistory: this._onChangeUndoHistory, + trashesAnnotations: this._trashesAnnotations, adjustTextAnnotationPosition: (annotation, option) => { return this._primaryView.adjustTextAnnotationPosition(annotation, option); } @@ -951,6 +953,13 @@ class Reader { this._annotationManager.unsetAnnotations(ids); } + // Called when the client permanently deletes annotations that are no longer + // in the view (e.g. erased from the trash), so history points referencing + // them can't be replayed and recreate them + clearAnnotationsHistory(ids) { + this._annotationManager.clearHistoryForAnnotations(ids); + } + openContextMenu(params) { this._onBringReaderToFront?.(true); this._updateState({ contextMenu: params }); From 11e3e6a8a737f09835c3ff1ec0379663b9e7cc9d Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Mon, 17 Aug 2026 13:39:14 -0700 Subject: [PATCH 2/3] fold clearAnnotationsHistory into unsetAnnotations If permanentlyDeleted flag is passed, annotations will be removed from history after being removed from the annotations array. --- src/common/annotation-manager.js | 19 ++++++++----------- src/common/reader.js | 11 ++--------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/common/annotation-manager.js b/src/common/annotation-manager.js index c6785a6b..e01b19ac 100644 --- a/src/common/annotation-manager.js +++ b/src/common/annotation-manager.js @@ -63,14 +63,17 @@ class AnnotationManager { } // Called when deletions come from the client side - unsetAnnotations(ids) { + unsetAnnotations(ids, permanentlyDeleted) { // Deletions we haven't applied yet are outside changes that our history // can no longer be replayed over. Ones we have applied are our own - // deletions coming back to us, and undoing them is still valid. - let externalIDs = ids.filter(id => this._annotations.some(x => x.id === id)); + // deletions coming back to us, and undoing them is still valid, unless + // the annotations were permanently deleted. + let clearIDs = permanentlyDeleted + ? ids + : ids.filter(id => this._annotations.some(x => x.id === id)); this._annotations = this._annotations.filter(x => !ids.includes(x.id)); - if (externalIDs.length) { - this._clearInterferingHistory(externalIDs); + if (clearIDs.length) { + this._clearInterferingHistory(clearIDs); } this.render(); } @@ -659,12 +662,6 @@ class AnnotationManager { return true; } - // Drops history points for annotations that have left the reader so - // that erased annotations cannot be brought back. - clearHistoryForAnnotations(ids) { - this._clearInterferingHistory(ids); - } - _clearInterferingHistory(affectedAnnotationIDs) { for (let i = this._undoStack.length - 1; i >= 0; i--) { if (affectedAnnotationIDs.some(id => this._undoStack[i].annotations.has(id))) { diff --git a/src/common/reader.js b/src/common/reader.js index b1375f10..86b5583f 100644 --- a/src/common/reader.js +++ b/src/common/reader.js @@ -949,15 +949,8 @@ class Reader { this._annotationManager.setAnnotations(annotations); } - unsetAnnotations(ids) { - this._annotationManager.unsetAnnotations(ids); - } - - // Called when the client permanently deletes annotations that are no longer - // in the view (e.g. erased from the trash), so history points referencing - // them can't be replayed and recreate them - clearAnnotationsHistory(ids) { - this._annotationManager.clearHistoryForAnnotations(ids); + unsetAnnotations(ids, permanentlyDeleted) { + this._annotationManager.unsetAnnotations(ids, permanentlyDeleted); } openContextMenu(params) { From 0105db721a81139b608847ae2cc657190694b8c1 Mon Sep 17 00:00:00 2001 From: abaevbog Date: Mon, 17 Aug 2026 13:43:50 -0700 Subject: [PATCH 3/3] Clarify comment Co-authored-by: Abe Jellinek <1770299+AbeJellinek@users.noreply.github.com> --- src/common/annotation-manager.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/common/annotation-manager.js b/src/common/annotation-manager.js index e01b19ac..d5f82f5a 100644 --- a/src/common/annotation-manager.js +++ b/src/common/annotation-manager.js @@ -605,9 +605,10 @@ class AnnotationManager { if (annotation) { annotation.dateModified = (new Date()).toISOString(); } - // Assign new id when undeleting to reduce sync conflicts. Clients that - // trash annotations keep the deleted one around, so undeleting restores - // it in place and the id has to stay the same for the client to find it. + // Assign a new ID when undeleting to reduce sync conflicts, + // except if the client trashes instead of permanently deleting - + // in that case, keep the old ID so the client can match the + // trashed annotation and untrash it. if (!prevAnnotation && !this._trashesAnnotations) { let newID = this._generateObjectKey(); mapping.set(annotation.id, newID);