From 325f12d551839855cd37345988a0dc6d57efb004 Mon Sep 17 00:00:00 2001 From: Tom Najdek Date: Thu, 5 Mar 2026 02:08:42 +0100 Subject: [PATCH] Add support for undo/redo --- chrome/content/zotero/collectionTree.jsx | 30 +- .../content/zotero/collectionViewItemTree.jsx | 62 +- .../containers/tagSelectorContainer.jsx | 7 +- chrome/content/zotero/elements/abstractBox.js | 8 +- .../content/zotero/elements/attachmentBox.js | 8 +- chrome/content/zotero/elements/itemBox.js | 67 +- .../content/zotero/elements/itemPaneHeader.js | 18 +- .../elements/librariesCollectionsBox.js | 5 +- chrome/content/zotero/elements/relatedBox.js | 2 + chrome/content/zotero/elements/tagsBox.js | 31 +- chrome/content/zotero/mergeItems.mjs | 4 + .../content/zotero/standalone/standalone.js | 42 + .../content/zotero/xpcom/data/collection.js | 13 + .../content/zotero/xpcom/data/dataObject.js | 74 +- chrome/content/zotero/xpcom/data/item.js | 109 ++ chrome/content/zotero/xpcom/data/items.js | 32 +- chrome/content/zotero/xpcom/data/tags.js | 12 + chrome/content/zotero/xpcom/reader.js | 5 +- .../content/zotero/xpcom/sync/syncEngine.js | 7 +- chrome/content/zotero/xpcom/sync/syncLocal.js | 23 +- .../content/zotero/xpcom/sync/syncRunner.js | 21 +- chrome/content/zotero/xpcom/undoHistory.js | 385 ++++++ chrome/content/zotero/xpcom/zotero.js | 6 + chrome/content/zotero/zotero.mjs | 1 + chrome/content/zotero/zoteroPane.js | 49 +- chrome/locale/en-US/zotero/zotero.ftl | 85 ++ defaults/preferences/zotero.js | 1 + test/tests/itemPaneTest.js | 136 +++ test/tests/undoHistoryTest.js | 1049 +++++++++++++++++ 29 files changed, 2207 insertions(+), 85 deletions(-) create mode 100644 chrome/content/zotero/xpcom/undoHistory.js create mode 100644 test/tests/undoHistoryTest.js diff --git a/chrome/content/zotero/collectionTree.jsx b/chrome/content/zotero/collectionTree.jsx index 90e2f1cad6..2438490942 100644 --- a/chrome/content/zotero/collectionTree.jsx +++ b/chrome/content/zotero/collectionTree.jsx @@ -275,7 +275,7 @@ var CollectionTree = class CollectionTree extends LibraryTree { if (!treeRow.editingName) return; treeRow.ref.name = treeRow.editingName; delete treeRow.editingName; - await treeRow.ref.saveTx(); + await treeRow.ref.saveTx({ undoAction: 'undo-action-rename-collection' }); window.Zotero_Tabs.rename("zotero-pane", treeRow.ref.name); } @@ -1312,10 +1312,17 @@ var CollectionTree = class CollectionTree extends LibraryTree { } treeRow.ref.deleted = true; if (treeRow.isCollection()) { - await treeRow.ref.saveTx({ deleteItems }); + await treeRow.ref.saveTx({ + deleteItems, + undoAction: 'undo-action-trash-collection', + undoActionArgs: { count: 1 } + }); return; } - await treeRow.ref.saveTx(); + await treeRow.ref.saveTx({ + undoAction: 'undo-action-trash-search', + undoActionArgs: { count: 1 } + }); } unregister() { @@ -2192,16 +2199,21 @@ var CollectionTree = class CollectionTree extends LibraryTree { // Dropping items, collections, or searches into trash if (targetTreeRow.isTrash()) { let objects = []; + let undoAction; if (dataType == 'zotero/collection') { objects = await Zotero.Collections.getAsync(data); + undoAction = 'undo-action-trash-collection'; } else if (dataType == 'zotero/search') { objects = await Zotero.Searches.getAsync(data); + undoAction = 'undo-action-trash-search'; } else if (dataType == 'zotero/item') { objects = await Zotero.Items.getAsync(data); + undoAction = 'undo-action-trash'; } await Zotero.DB.executeTransaction(async function () { + Zotero.UndoHistory.stageAction(undoAction, { count: objects.length }); for (let obj of objects) { obj.deleted = true; await obj.save(); @@ -2228,7 +2240,7 @@ var CollectionTree = class CollectionTree extends LibraryTree { // Collection drag within a library else { droppedCollection.parentID = targetCollectionID; - await droppedCollection.saveTx(); + await droppedCollection.saveTx({ undoAction: 'undo-action-move-collection' }); } } else if (dataType == 'zotero/item') { @@ -2300,6 +2312,16 @@ var CollectionTree = class CollectionTree extends LibraryTree { await Zotero.DB.executeTransaction(async function () { let collection = await Zotero.Collections.getAsync(targetCollectionID); await collection.addItems(ids); + // If moving, remove from source in the same + // transaction so it's a single undo step + if (dropEffect == 'move' && toMove.length + && sourceTreeRow && sourceTreeRow.isCollection()) { + await sourceTreeRow.ref.removeItems(toMove); + toMove = []; + Zotero.UndoHistory.stageAction( + 'undo-action-move-to-collection', { count: ids.length } + ); + } }.bind(this)); } else if (targetTreeRow.isPublications()) { diff --git a/chrome/content/zotero/collectionViewItemTree.jsx b/chrome/content/zotero/collectionViewItemTree.jsx index 7ed1ed2031..d5c1fd9c39 100644 --- a/chrome/content/zotero/collectionViewItemTree.jsx +++ b/chrome/content/zotero/collectionViewItemTree.jsx @@ -1330,6 +1330,11 @@ class CollectionViewItemTree extends ItemTree { // canDrop() limits this to child items var rowItem = this.getRow(row).ref; // the item we are dragging over await Zotero.DB.executeTransaction(async function () { + Zotero.UndoHistory.stageAction( + 'undo-action-change-parent-item', + { count: items.length } + ); + for (let i = 0; i < items.length; i++) { let item = items[i]; item.parentID = rowItem.id; @@ -1341,33 +1346,34 @@ class CollectionViewItemTree extends ItemTree { // Dropped outside of a row else { // Remove from parent and make top-level + // (canDropCheck guarantees all items here are children with parents) if (collectionTreeRow.isLibrary(true)) { await Zotero.DB.executeTransaction(async function () { + Zotero.UndoHistory.stageAction( + 'undo-action-convert-to-standalone', + { count: items.length } + ); + for (let i = 0; i < items.length; i++) { let item = items[i]; - if (!item.isRegularItem()) { - item.parentID = false; - await item.save(); - } + item.parentID = false; + await item.save(); } }); } // Add to collection else { await Zotero.DB.executeTransaction(async function () { + Zotero.UndoHistory.stageAction( + 'undo-action-convert-to-standalone', + { count: items.length } + ); + for (let i = 0; i < items.length; i++) { let item = items[i]; - var source = item.isRegularItem() ? false : item.parentItemID; - // Top-level item - if (source) { - item.parentID = false; - item.addToCollection(collectionTreeRow.ref.id); - await item.save(); - } - else { - item.addToCollection(collectionTreeRow.ref.id); - await item.save(); - } + item.parentID = false; + item.addToCollection(collectionTreeRow.ref.id); + await item.save(); toMove.push(item.id); } }); @@ -1646,15 +1652,21 @@ class CollectionViewItemTree extends ItemTree { // Async but no need to wait (async () => { - for (let item of items) { - if (tagRemove) { - item.removeTag(colorData.name); + await Zotero.DB.executeTransaction(async () => { + Zotero.UndoHistory.stageAction( + tagRemove ? 'undo-action-remove-tag' : 'undo-action-add-tag', + { count: items.length } + ); + for (let item of items) { + if (tagRemove) { + item.removeTag(colorData.name); + } + else { + item.addTag(colorData.name); + } + await item.save(); } - else { - item.addTag(colorData.name); - } - await item.saveTx(); - } + }); })(); // We handled this @@ -1787,6 +1799,10 @@ class CollectionViewItemTree extends ItemTree { } await Zotero.DB.executeTransaction(async () => { + Zotero.UndoHistory.stageAction( + 'undo-action-remove-from-collection', + { count: selectedItems.length } + ); for (let item of selectedItems) { for (let collectionID of collectionIDs) { item.removeFromCollection(collectionID); diff --git a/chrome/content/zotero/containers/tagSelectorContainer.jsx b/chrome/content/zotero/containers/tagSelectorContainer.jsx index 45afdc0736..5b2446b5d2 100644 --- a/chrome/content/zotero/containers/tagSelectorContainer.jsx +++ b/chrome/content/zotero/containers/tagSelectorContainer.jsx @@ -665,7 +665,11 @@ Zotero.TagSelector = class TagSelectorContainer extends React.PureComponent { ids = ids.split(','); var items = Zotero.Items.get(ids); var value = elem.textContent; - + + Zotero.UndoHistory.stageAction( + remove ? 'undo-action-remove-tag' : 'undo-action-add-tag', + { count: items.length } + ); for (let i=0; i { + Zotero.UndoHistory.stageAction('undo-action-split-tag'); for (const itemID of itemIDs) { const item = await Zotero.Items.getAsync(itemID); const tagType = item.getTagType(oldTagName); diff --git a/chrome/content/zotero/elements/abstractBox.js b/chrome/content/zotero/elements/abstractBox.js index ca8632dd3b..fb2cd73370 100644 --- a/chrome/content/zotero/elements/abstractBox.js +++ b/chrome/content/zotero/elements/abstractBox.js @@ -138,7 +138,13 @@ throw new Error('Item has not been added to library'); } this._item.setField('abstractNote', this._abstractField.value); - await this._item.saveTx(); + await this._item.saveTx({ + undoAction: 'undo-action-edit-field', + undoActionArgs: { + field: Zotero.ItemFields.getLocalizedString('abstractNote'), + count: 1 + } + }); } this._forceRenderAll(); } diff --git a/chrome/content/zotero/elements/attachmentBox.js b/chrome/content/zotero/elements/attachmentBox.js index 160b90328e..208da2fbb5 100644 --- a/chrome/content/zotero/elements/attachmentBox.js +++ b/chrome/content/zotero/elements/attachmentBox.js @@ -765,7 +765,13 @@ _handleTitleBlur = () => { this.item.setField('title', this._id('title').value); - this.item.saveTx(); + this.item.saveTx({ + undoAction: 'undo-action-edit-field', + undoActionArgs: { + field: Zotero.ItemFields.getLocalizedString('title'), + count: 1 + } + }); }; _handleFileNameFocus = () => { diff --git a/chrome/content/zotero/elements/itemBox.js b/chrome/content/zotero/elements/itemBox.js index a62ab93051..a965f9f1ad 100644 --- a/chrome/content/zotero/elements/itemBox.js +++ b/chrome/content/zotero/elements/itemBox.js @@ -422,7 +422,8 @@ this.renderCustomRows(ids); return; } - if (event == 'modify' && this.item?.id && ids.includes(this.item.id)) { + if (event == 'modify' && this.item?.id + && (ids.includes(this.item.id) || this._extraItems?.some(item => ids.includes(item.id)))) { this._forceRenderAll(); } if (event === 'select' && type === 'tab' && ids.length > 0) { @@ -773,7 +774,7 @@ optionsButton.addEventListener("click", onContextMenu); rowData.appendChild(optionsButton); // Options button is always created for focus management but if the field is empty, it is hidden - if (!val) optionsButton.hidden = true; + if (!val && !extraFieldValues.some(v => v)) optionsButton.hidden = true; } rowData.oncontextmenu = onContextMenu; @@ -1737,7 +1738,7 @@ firstName.sizeToContent(); lastName.sizeToContent(); this.modifyCreator(rowIndex, fields); - this.item.saveTx(); + this.item.saveTx({ undoAction: 'undo-action-edit-creator' }); } } @@ -1759,8 +1760,13 @@ return true; } + // Flush any pending field edits as a separate undo step + // before changing the item type if (this.saveOnEdit) { - await this.item.saveTx(); + await this.item.saveTx({ + undoAction: 'undo-action-edit-metadata', + undoActionArgs: { count: 1 } + }); } var fieldsToDelete = this.item.getFieldsNotInType(itemTypeID, true); @@ -1817,7 +1823,7 @@ this.item.setType(itemTypeID); if (this.saveOnEdit) { - await this.item.saveTx(); + await this.item.saveTx({ undoAction: 'undo-action-change-type' }); } else { this._forceRenderAll(); @@ -2095,7 +2101,7 @@ return; } this.item.removeCreator(index); - await this.item.saveTx(); + await this.item.saveTx({ undoAction: 'undo-action-remove-creator' }); } removeUnsavedCreatorRow(onlyIfEmpty = false) { @@ -2300,11 +2306,11 @@ var fields = this.getCreatorFields(row); fields[creatorField] = creator[creatorField]; fields[otherField] = creator[otherField]; - + this.modifyCreator(creatorIndex, fields); if (this.saveOnEdit) { this.ignoreBlur = true; - this.item.saveTx().then(() => { + this.item.saveTx({ undoAction: 'undo-action-edit-creator' }).then(() => { this.ignoreBlur = false; }); } @@ -2395,7 +2401,7 @@ this._selectField = `itembox-field-value-creator-${newCreator.position}-lastName`; if (this.saveOnEdit) { - this.item.saveTx(); + this.item.saveTx({ undoAction: 'undo-action-edit-creator' }); } } } @@ -2468,10 +2474,14 @@ var [field, creatorIndex, creatorField] = fieldName.split('-'); // Creator fields + let isCreatorField = false; + let isCreatorUnsaved = false; if (field == 'creator') { + isCreatorField = true; var row = textbox.closest('.meta-row'); var otherFields = this.getCreatorFields(row); + isCreatorUnsaved = otherFields.isUnsaved; otherFields[creatorField] = value; this.modifyCreator(creatorIndex, otherFields); @@ -2545,7 +2555,20 @@ } if (this.saveOnEdit) { - await this._saveItems(); + let saveOptions = {}; + if (isCreatorField) { + saveOptions.undoAction = isCreatorUnsaved + ? 'undo-action-add-creator' + : 'undo-action-edit-creator'; + } + else { + saveOptions.undoAction = 'undo-action-edit-field'; + saveOptions.undoActionArgs = { + field: Zotero.ItemFields.getLocalizedString(fieldName), + count: 1 + this._extraItems.length + }; + } + await this._saveItems(saveOptions); } } @@ -2581,7 +2604,7 @@ } } - async _saveItems() { + async _saveItems(saveOptions = {}) { // Cache item and extra items to avoid a race condition where, after `hideEditor`, // while we yield for `await Zotero.DB.executeTransaction`, itemBox is rendered for // the new item and this.item is no longer relevant @@ -2589,9 +2612,9 @@ let extraItems = this._extraItems; await Zotero.DB.executeTransaction(async () => { - await item.save(); + await item.save(saveOptions); for (let extraItem of extraItems) { - await extraItem.save(); + await extraItem.save(saveOptions); } }); if (extraItems.length) { @@ -2620,10 +2643,16 @@ }); if (this.saveOnEdit) { - await this._saveItems(); + await this._saveItems({ + undoAction: 'undo-action-edit-field', + undoActionArgs: { + field: Zotero.ItemFields.getLocalizedString(fieldName), + count: 1 + this._extraItems.length + } + }); } } - + // Make sure that irrelevant creators +/- buttons are disabled _updateCreatorButtonsStatus() { @@ -2723,7 +2752,7 @@ this.modifyCreator(creatorIndex, fields); if (this.saveOnEdit) { - await this.item.saveTx(); + await this.item.saveTx({ undoAction: 'undo-action-edit-creator' }); } } @@ -2746,7 +2775,7 @@ var fields = this.getCreatorFields(row); this.modifyCreator(creatorIndex, fields); if (this.saveOnEdit) { - await this.item.saveTx(); + await this.item.saveTx({ undoAction: 'undo-action-edit-creator' }); } } @@ -2860,7 +2889,7 @@ this.item.setCreator(i, creators[i]); } if (this.saveOnEdit && !skipSave) { - this.item.saveTx(); + this.item.saveTx({ undoAction: 'undo-action-reorder-creator' }); } } @@ -3216,7 +3245,7 @@ this.modifyCreator(index, fields); if (this.saveOnEdit) { - await this.item.saveTx(); + await this.item.saveTx({ undoAction: 'undo-action-edit-creator' }); } }; diff --git a/chrome/content/zotero/elements/itemPaneHeader.js b/chrome/content/zotero/elements/itemPaneHeader.js index 2b5c8fda8f..be8e18cb48 100644 --- a/chrome/content/zotero/elements/itemPaneHeader.js +++ b/chrome/content/zotero/elements/itemPaneHeader.js @@ -201,9 +201,15 @@ if (newValue.toLowerCase().startsWith(shortTitleVal.toLowerCase())) { this._item.setField('shortTitle', newValue.substring(0, shortTitleVal.length)); } - await this._item.saveTx(); + await this._item.saveTx({ + undoAction: 'undo-action-edit-field', + undoActionArgs: { + field: Zotero.ItemFields.getLocalizedString(this._titleFieldID), + count: 1 + } + }); } - + async save() { if (!this.editable) { return; @@ -213,7 +219,13 @@ throw new Error('Item has not been added to library'); } this._item.setField(this._titleFieldID, this.titleField.value); - await this._item.saveTx(); + await this._item.saveTx({ + undoAction: 'undo-action-edit-field', + undoActionArgs: { + field: Zotero.ItemFields.getLocalizedString(this._titleFieldID), + count: 1 + } + }); } this._forceRenderAll(); } diff --git a/chrome/content/zotero/elements/librariesCollectionsBox.js b/chrome/content/zotero/elements/librariesCollectionsBox.js index 5eb173b07e..79dbd01718 100644 --- a/chrome/content/zotero/elements/librariesCollectionsBox.js +++ b/chrome/content/zotero/elements/librariesCollectionsBox.js @@ -148,7 +148,10 @@ import { getCSSIcon } from 'components/icons'; Zotero.getString('pane.items.removeFromOther', [obj.name]) )) { contextItem.removeFromCollection(obj.id); - contextItem.saveTx(); + contextItem.saveTx({ + undoAction: 'undo-action-remove-from-collection', + undoActionArgs: { count: 1 } + }); } }); row.append(remove); diff --git a/chrome/content/zotero/elements/relatedBox.js b/chrome/content/zotero/elements/relatedBox.js index 6f92f14a64..6024d1f613 100644 --- a/chrome/content/zotero/elements/relatedBox.js +++ b/chrome/content/zotero/elements/relatedBox.js @@ -178,6 +178,7 @@ import { getCSSItemTypeIcon } from 'components/icons'; return; } await Zotero.DB.executeTransaction(async () => { + Zotero.UndoHistory.stageAction('undo-action-add-related'); for (let relItem of relItems) { if (this._item.addRelatedItem(relItem)) { await this._item.save({ @@ -197,6 +198,7 @@ import { getCSSItemTypeIcon } from 'components/icons'; let item = await Zotero.Items.getAsync(id); if (item) { await Zotero.DB.executeTransaction(async () => { + Zotero.UndoHistory.stageAction('undo-action-remove-related'); if (this._item.removeRelatedItem(item)) { await this._item.save({ skipDateModifiedUpdate: true diff --git a/chrome/content/zotero/elements/tagsBox.js b/chrome/content/zotero/elements/tagsBox.js index 9e36ee5e02..03e4b95d71 100644 --- a/chrome/content/zotero/elements/tagsBox.js +++ b/chrome/content/zotero/elements/tagsBox.js @@ -246,6 +246,7 @@ this.remove(tagName); try { item.removeTag(tagName); + this._pendingRemovalCount++; // Save item after a debounce to avoid triggering multiple // save operations. If there are many tags in the library, // db transaction may not keep up with UI changes, and cause @@ -466,7 +467,7 @@ this.add(value); try { this.item.replaceTag(oldValue, value); - await this.item.saveTx(); + await this.item.saveTx({ undoAction: 'undo-action-change-tag' }); } catch (e) { this._forceRenderAll(); @@ -483,7 +484,10 @@ nextRowElem?.focus(); } this.item.removeTag(oldValue); - await this.item.saveTx(); + await this.item.saveTx({ + undoAction: 'undo-action-remove-tag', + undoActionArgs: { count: 1 } + }); } catch (e) { this._forceRenderAll(); @@ -503,7 +507,10 @@ } tags.forEach(tag => this.item.addTag(tag)); - await this.item.saveTx(); + await this.item.saveTx({ + undoAction: 'undo-action-add-tag', + undoActionArgs: { count: 1 } + }); } // Single tag at end else { @@ -518,7 +525,10 @@ this.add(value); this.item.addTag(value); try { - await this.item.saveTx(); + await this.item.saveTx({ + undoAction: 'undo-action-add-tag', + undoActionArgs: { count: 1 } + }); } catch (e) { this._forceRenderAll(); @@ -602,7 +612,9 @@ removeAll = () => { if (Services.prompt.confirm(null, "", Zotero.getString('pane.item.tags.removeAll'))) { this.item.setTags([]); - this.item.saveTx(); + this.item.saveTx({ + undoAction: 'undo-action-remove-all-tags' + }); } }; @@ -652,8 +664,15 @@ } } + _pendingRemovalCount = 0; + _saveItemDebounced = Zotero.Utilities.debounce(async (item) => { - await item.saveTx(); + let count = this._pendingRemovalCount; + this._pendingRemovalCount = 0; + await item.saveTx({ + undoAction: 'undo-action-remove-tags-from-item', + undoActionArgs: { count } + }); }); _id(id) { diff --git a/chrome/content/zotero/mergeItems.mjs b/chrome/content/zotero/mergeItems.mjs index 2ed9518f10..496b4dd844 100644 --- a/chrome/content/zotero/mergeItems.mjs +++ b/chrome/content/zotero/mergeItems.mjs @@ -4,6 +4,10 @@ export function mergeItems(item, otherItems) { Zotero.debug("Merging items"); return Zotero.DB.executeTransaction(async function () { + Zotero.UndoHistory.stageAction( + 'undo-action-merge-items', + { count: otherItems.length + 1 } + ); var toSave = {}; toSave[item.id] = item; diff --git a/chrome/content/zotero/standalone/standalone.js b/chrome/content/zotero/standalone/standalone.js index a7723e998b..48a76f9937 100644 --- a/chrome/content/zotero/standalone/standalone.js +++ b/chrome/content/zotero/standalone/standalone.js @@ -237,9 +237,51 @@ const ZoteroStandalone = new function () { this.updateQuickCopyOptions(); // goUpdateGlobalEditMenuItems(true) is necessary to update Edit menu when contenteditable is focused window.goUpdateGlobalEditMenuItems(true); + this._updateUndoRedoLabels(); this.onUpdateCustomMenus(event, 'edit'); }; + + this._updateUndoRedoLabels = function () { + let undoItem = document.getElementById('menu_undo'); + let redoItem = document.getElementById('menu_redo'); + if (!undoItem || !redoItem) return; + + // When a native text-editing controller handles undo/redo + // (e.g. focused input), show generic labels and let it take over + let nativeUndo = Zotero.UndoHistory.hasNativeUndo(document); + let nativeRedo = Zotero.UndoHistory.hasNativeRedo(document); + + let undoAction = !nativeUndo && Zotero.UndoHistory.getUndoAction(); + if (undoAction) { + let actionLabel = Zotero.ftl.formatValueSync( + undoAction.action, undoAction.actionArgs || undefined + ); + let fullLabel = Zotero.ftl.formatValueSync( + 'menu-edit-undo-action', { action: actionLabel } + ); + undoItem.removeAttribute('data-l10n-id'); + undoItem.setAttribute('label', fullLabel); + } + else { + document.l10n.setAttributes(undoItem, 'text-action-undo'); + } + + let redoAction = !nativeRedo && Zotero.UndoHistory.getRedoAction(); + if (redoAction) { + let actionLabel = Zotero.ftl.formatValueSync( + redoAction.action, redoAction.actionArgs || undefined + ); + let fullLabel = Zotero.ftl.formatValueSync( + 'menu-edit-redo-action', { action: actionLabel } + ); + redoItem.removeAttribute('data-l10n-id'); + redoItem.setAttribute('label', fullLabel); + } + else { + document.l10n.setAttributes(redoItem, 'text-action-redo'); + } + }; /** * Builds new item menu diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index 2c7b4f9f2b..a68f150dc3 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -386,6 +386,7 @@ Zotero.Collection.prototype._finalizeSave = async function (env) { }; + /** * @param {Number} itemID * @return {Promise} @@ -616,6 +617,18 @@ Zotero.Collection.prototype.trash = async function (env) { if (env.options && env.options.skipDeleteLog) { env.notifierData[c.id].skipDeleteLog = true; } + // Record undo data for descendent collections + if (Zotero.UndoHistory && !c.deleted) { + Zotero.UndoHistory.stageChange({ + objectType: 'collection', + id: c.id, + libraryID: c.libraryID, + key: c.key, + fields: { + deleted: { old: false, new: true } + } + }); + } } } // Descendent items diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js index b59ba142cb..56fb3168c3 100644 --- a/chrome/content/zotero/xpcom/data/dataObject.js +++ b/chrome/content/zotero/xpcom/data/dataObject.js @@ -884,6 +884,54 @@ Zotero.DataObject.prototype._markForReload = function (dataType) { } +Zotero.DataObject.UNDO_SKIP_FIELDS = new Set(['version', 'synced', 'clientDateModified', 'dateModified']); + +/** + * Build a change record for UndoHistory from the current pending changes. + * Called during save() before _saveData() clears change tracking. + * + * @return {Object|null} - ChangeRecord or null if nothing undoable + */ +Zotero.DataObject.prototype._getUndoData = function () { + let skipFields = Zotero.DataObject.UNDO_SKIP_FIELDS; + let fields = {}; + + // Fields tracked via _previousData (old-style: old value stored) + for (let field of Object.keys(this._previousData)) { + if (skipFields.has(field)) continue; + // Skip non-scalar fields like relations + if (typeof this._previousData[field] === 'object' && this._previousData[field] !== null) { + continue; + } + fields[field] = { + old: this._previousData[field], + new: this['_' + field] + }; + } + + // Fields tracked via _changedData (new-style: new value stored) + for (let field of Object.keys(this._changedData)) { + if (skipFields.has(field)) continue; + if (field === 'deleted') { + fields[field] = { + old: this._deleted, + new: this._changedData[field] + }; + } + } + + if (!Object.keys(fields).length) return null; + + return { + objectType: this._objectType, + id: this._id, + libraryID: this._libraryID, + key: this._key, + fields + }; +}; + + /** * @param {String} [op='edit'] - Operation to check; if not provided, check edit privileges for * library @@ -907,6 +955,11 @@ Zotero.DataObject.prototype.isEditable = function (_op = 'edit') { * @param {Boolean} [options.skipNotifier] - Don't trigger Zotero.Notifier events * @param {Boolean} [options.skipSelect] - Don't select object automatically in trees * @param {Boolean} [options.skipSyncedUpdate] - Don't automatically set 'synced' to false + * @param {String} [options.undoAction] - Fluent message ID for the undo entry's action label + * (e.g. 'undo-action-edit-creator'); without this the save + * is captured but discarded at commit + * @param {Object} [options.undoActionArgs] - Fluent message arguments for the action label + * (e.g. { count: 3 }) * @return {Promise} Promise for itemID of new item, * TRUE on item update, or FALSE if item was unchanged */ @@ -969,7 +1022,14 @@ Zotero.DataObject.prototype.save = async function (options = {}) { env.notifierData.changed[field] = this['_' + field]; } } - + + // Capture undo data before _saveData clears change tracking. + // Capture is unconditional for non-isNew saves; whether it lands on the + // undo stack depends on a stageAction() call within the same transaction. + if (Zotero.UndoHistory && !env.isNew) { + env.undoData = this._getUndoData(); + } + // Create transaction let result if (env.options.tx) { @@ -977,6 +1037,12 @@ Zotero.DataObject.prototype.save = async function (options = {}) { Zotero.DataObject.prototype._saveData.call(this, env); await this._saveData(env); await Zotero.DataObject.prototype._finalizeSave.call(this, env); + if (env.undoData) { + Zotero.UndoHistory.stageChange(env.undoData); + if (env.options.undoAction) { + Zotero.UndoHistory.stageAction(env.options.undoAction, env.options.undoActionArgs); + } + } return this._finalizeSave(env); }.bind(this), env.transactionOptions); } @@ -987,6 +1053,12 @@ Zotero.DataObject.prototype.save = async function (options = {}) { await this._saveData(env); await Zotero.DataObject.prototype._finalizeSave.call(this, env); result = this._finalizeSave(env); + if (env.undoData) { + Zotero.UndoHistory.stageChange(env.undoData); + if (env.options.undoAction) { + Zotero.UndoHistory.stageAction(env.options.undoAction, env.options.undoActionArgs); + } + } } this._postSave(env); return result; diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 99669a0931..06f08344bf 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -880,6 +880,115 @@ Zotero.Item.prototype.setField = function (field, value, loadIn) { return true; } +/** + * Override to correctly resolve item data fields via _itemData[fieldID] + */ +Zotero.Item.prototype._getUndoData = function () { + let skipFields = Zotero.DataObject.UNDO_SKIP_FIELDS; + let fields = {}; + + // Fields tracked via _previousData + for (let field of Object.keys(this._previousData)) { + if (skipFields.has(field)) continue; + // 'itemType' is a derived name, not directly settable -- handled below as itemTypeID + if (field === 'itemType') continue; + // Collections are an array but need explicit undo tracking + if (field === 'collections') { + fields[field] = { + old: this._previousData[field], + new: this._collections + }; + continue; + } + if (field === 'note') { + fields[field] = { + old: this._previousData[field], + new: this._noteText + }; + continue; + } + if (field === 'relations') { + fields[field] = { + old: this._previousData[field], + new: this._relations.map(r => [...r]) + }; + continue; + } + if (typeof this._previousData[field] === 'object' && this._previousData[field] !== null) { + continue; + } + + let fieldID = Zotero.ItemFields.getID(field); + if (fieldID) { + // Item data field -- new value is in _itemData. + // After a type change, lost fields are no longer in _itemData. + let newValue = this._itemData[fieldID]; + fields[field] = { + old: this._previousData[field], + new: newValue !== undefined ? newValue : false + }; + } + else { + // Primary data field -- new value is on the instance property + fields[field] = { + old: this._previousData[field], + new: this['_' + field] + }; + } + } + + // Detect item type change and store with numeric IDs + if (this._changed.primaryData && this._changed.primaryData.itemTypeID + && this._previousData.itemType) { + fields.itemTypeID = { + old: Zotero.ItemTypes.getID(this._previousData.itemType), + new: this._itemTypeID + }; + } + + // Fields tracked via _changedData (e.g. deleted, tags) + for (let field of Object.keys(this._changedData)) { + if (skipFields.has(field)) continue; + if (field === 'deleted') { + fields[field] = { + old: this._deleted, + new: this._changedData[field] + }; + } + else if (field === 'tags') { + fields[field] = { + old: this._tags, + new: this._changedData[field] + }; + } + } + + // Creators tracked via _changed.creators + if (this._changed.creators) { + // Old creators were saved in _previousData.creators by _markFieldChange + let oldCreators = this._previousData.creators || {}; + let newCreators = {}; + for (let i = 0; i < this._creators.length; i++) { + newCreators[i] = Object.assign({}, this._creators[i]); + } + fields.creators = { + old: oldCreators, + new: newCreators + }; + } + + if (!Object.keys(fields).length) return null; + + return { + objectType: this._objectType, + id: this._id, + libraryID: this._libraryID, + key: this._key, + fields + }; +}; + + /* * Get the title for an item for display in the interface * diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index 0f869f7bbc..3a5fe3e4cc 100644 --- a/chrome/content/zotero/xpcom/data/items.js +++ b/chrome/content/zotero/xpcom/data/items.js @@ -980,12 +980,13 @@ Zotero.Items = function () { }; - this.trash = async function (ids) { + this.trash = async function (ids, options = {}) { Zotero.DB.requireTransaction(); - + var libraryIDs = new Set(); ids = Zotero.flattenArguments(ids); var items = []; + var undoableCount = 0; for (let id of ids) { let item = this.get(id); if (!item) { @@ -993,18 +994,35 @@ Zotero.Items = function () { Zotero.Notifier.queue('trash', 'item', id); continue; } - + if (!item.isEditable()) { throw new Error(item._ObjectType + " " + item.libraryKey + " is not editable"); } - + if (!Zotero.Libraries.get(item.libraryID).hasTrash) { throw new Error(Zotero.Libraries.getName(item.libraryID) + " does not have a trash"); } - + + // Record undo data before modifying state + if (Zotero.UndoHistory && !item.deleted) { + Zotero.UndoHistory.stageChange({ + objectType: 'item', + id: item.id, + libraryID: item.libraryID, + key: item.key, + fields: { + deleted: { old: false, new: true } + } + }); + undoableCount++; + } + items.push(item); libraryIDs.add(item.libraryID); } + if (Zotero.UndoHistory && undoableCount) { + Zotero.UndoHistory.stageAction('undo-action-trash', { count: undoableCount }); + } var parentItemIDs = new Set(); items.forEach(item => { @@ -1055,9 +1073,9 @@ Zotero.Items = function () { }; - this.trashTx = function (ids) { + this.trashTx = function (ids, options) { return Zotero.DB.executeTransaction(async function () { - return this.trash(ids); + return this.trash(ids, options); }.bind(this)); } diff --git a/chrome/content/zotero/xpcom/data/tags.js b/chrome/content/zotero/xpcom/data/tags.js index 8003462dd5..9338f47255 100644 --- a/chrome/content/zotero/xpcom/data/tags.js +++ b/chrome/content/zotero/xpcom/data/tags.js @@ -800,6 +800,10 @@ Zotero.Tags = new function () { return Zotero.DB.executeTransaction(async function () { // If all items already have the tag, remove it from all items if (tagID && items.every(x => x.hasTag(tagName))) { + Zotero.UndoHistory.stageAction( + 'undo-action-remove-tag', + { count: items.length } + ); for (let item of items) { if (item.removeTag(tagName)) { await item.save(); @@ -809,6 +813,10 @@ Zotero.Tags = new function () { } // Otherwise add to all items else { + Zotero.UndoHistory.stageAction( + 'undo-action-add-tag', + { count: items.length } + ); for (let item of items) { if (item.addTag(tagName)) { await item.save(); @@ -825,6 +833,10 @@ Zotero.Tags = new function () { */ this.removeColoredTagsFromItems = async function (items) { return Zotero.DB.executeTransaction(async function () { + Zotero.UndoHistory.stageAction( + 'undo-action-remove-tag', + { count: items.length } + ); for (let item of items) { let colors = this.getColors(item.libraryID); let tags = item.getTags(); diff --git a/chrome/content/zotero/xpcom/reader.js b/chrome/content/zotero/xpcom/reader.js index b4cecf3d70..a62da149e0 100644 --- a/chrome/content/zotero/xpcom/reader.js +++ b/chrome/content/zotero/xpcom/reader.js @@ -136,7 +136,10 @@ class ReaderInstance { let item = Zotero.Items.getByLibraryAndKey(libraryID, key); if (item && item.isEditable()) { item.annotationColor = color; - await item.saveTx({ skipDateModifiedUpdate: true, notifierQueue }); + await item.saveTx({ + skipDateModifiedUpdate: true, + notifierQueue + }); } } } diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index 3c939a7cc9..26b4bcb22d 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -804,6 +804,7 @@ Zotero.Sync.Data.Engine.prototype._restoreRestoredCollectionItems = async functi if (o.deleted) { o.deleted = false await o.saveTx(); + Zotero.Sync.Data.Local.markRemoteChangesApplied(); } } else { @@ -816,6 +817,7 @@ Zotero.Sync.Data.Engine.prototype._restoreRestoredCollectionItems = async functi + `to restored collection ${collection.libraryKey}`); await Zotero.DB.executeTransaction(async function () { await collection.addItems(addToCollection); + Zotero.Sync.Data.Local.markRemoteChangesApplied(); }.bind(this)); } if (addToQueue.length) { @@ -911,6 +913,7 @@ Zotero.Sync.Data.Engine.prototype._downloadDeletions = async function (since, ne await obj.eraseTx({ skipDeleteLog: true }); + Zotero.Sync.Data.Local.markRemoteChangesApplied(); continue; } conflicts.push({ @@ -960,12 +963,13 @@ Zotero.Sync.Data.Engine.prototype._downloadDeletions = async function (since, ne await obj.erase({ skipEditCheck: true }); + Zotero.Sync.Data.Local.markRemoteChangesApplied(); } }.bind(this)); }.bind(this) ); } - + if (toDelete.length) { await Zotero.Utilities.Internal.forEachChunkAsync( toDelete, @@ -977,6 +981,7 @@ Zotero.Sync.Data.Engine.prototype._downloadDeletions = async function (since, ne skipEditCheck: true, skipDeleteLog: true }); + Zotero.Sync.Data.Local.markRemoteChangesApplied(); } }); } diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index d5a50384fd..61a05f4b41 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -33,7 +33,21 @@ Zotero.Sync.Data.Local = { _loginManagerRealm: 'Zotero Web API', _lastSyncTime: null, _lastClassicSyncTime: null, - + // Item/Collection-only -- synced settings can't affect the undo stack + _remoteChangesApplied: false, + + get remoteChangesApplied() { + return this._remoteChangesApplied; + }, + + resetRemoteChangesApplied: function () { + this._remoteChangesApplied = false; + }, + + markRemoteChangesApplied: function () { + this._remoteChangesApplied = true; + }, + init: async function () { await this._loadLastSyncTime(); if (!_lastSyncTime) { @@ -289,7 +303,7 @@ Zotero.Sync.Data.Local = { var library = Zotero.Libraries.get(libraryID); library.libraryVersion = -1; await library.saveTx(); - + await this.resetUnsyncedLibraryFiles(libraryID); }, @@ -349,8 +363,9 @@ Zotero.Sync.Data.Local = { skipDeleteLog: true } ); + this.markRemoteChangesApplied(); } - + // Deleted objects keys = await Zotero.Sync.Data.Local.getDeleted(objectType, libraryID); await this.removeObjectsFromDeleteLog(objectType, libraryID, keys); @@ -1388,6 +1403,7 @@ Zotero.Sync.Data.Local = { await obj.erase({ notifierQueue }); + Zotero.Sync.Data.Local.markRemoteChangesApplied(); } catch (e) { results.push({ @@ -1561,6 +1577,7 @@ Zotero.Sync.Data.Local = { obj.synced = true; } await obj.save(saveOptions); + this.markRemoteChangesApplied(); let cacheJSON = options.cacheObject ? options.cacheObject : json.data; await this.saveCacheObject(obj.objectType, obj.libraryID, cacheJSON); // Delete older versions of the object in the cache diff --git a/chrome/content/zotero/xpcom/sync/syncRunner.js b/chrome/content/zotero/xpcom/sync/syncRunner.js index 7ac415f658..27ee18b55d 100644 --- a/chrome/content/zotero/xpcom/sync/syncRunner.js +++ b/chrome/content/zotero/xpcom/sync/syncRunner.js @@ -116,13 +116,13 @@ Zotero.Sync.Runner_Module = function (options = {}) { this.sync = Zotero.serial(function (options = {}) { return this._sync(options); }); - - + + this._sync = async function (options) { // Clear message list _errors = []; _tooltipMessages = []; - + // Shouldn't be possible because of serial() if (_syncInProgress) { let msg = Zotero.getString('sync.error.syncInProgress'); @@ -132,6 +132,10 @@ Zotero.Sync.Runner_Module = function (options = {}) { } _syncInProgress = true; _stopping = false; + + // Reset remote-change tracking for this sync; the undo stack is + // cleared lazily at the end only if remote mutations were applied. + Zotero.Sync.Data.Local.resetRemoteChangesApplied(); try { await Zotero.Notifier.trigger('start', 'sync', []); @@ -321,7 +325,14 @@ Zotero.Sync.Runner_Module = function (options = {}) { } finally { await this.end(options); - + + // Clear undo history if this iteration applied remote changes. + // Done before any restart/queued recursive call so the inner + // sync's reset doesn't lose the decision made here. + if (Zotero.Sync.Data.Local.remoteChangesApplied) { + Zotero.UndoHistory.clear(); + } + if (options.restartSync) { delete options.restartSync; Zotero.debug("Restarting sync"); @@ -334,7 +345,7 @@ Zotero.Sync.Runner_Module = function (options = {}) { await this._sync(JSON.parse(_queuedSyncOptions.shift())); return; } - + Zotero.debug("Done syncing"); Zotero.Notifier.trigger('finish', 'sync', librariesToSync || []); } diff --git a/chrome/content/zotero/xpcom/undoHistory.js b/chrome/content/zotero/xpcom/undoHistory.js new file mode 100644 index 0000000000..07f42a8b85 --- /dev/null +++ b/chrome/content/zotero/xpcom/undoHistory.js @@ -0,0 +1,385 @@ +/* + ***** BEGIN LICENSE BLOCK ***** + + Copyright © 2026 Corporation for Digital Scholarship + Vienna, Virginia, USA + https://digitalscholar.org + + This file is part of Zotero. + + Zotero is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Zotero is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with Zotero. If not, see . + + ***** END LICENSE BLOCK ***** +*/ + +/** + * In-memory undo/redo stack for DataObject field changes. + * + * Hooks into DB transaction lifecycle to batch all saves within a single + * executeTransaction() into one undo step. Only tracks modifications to + * existing objects. + * + * Capture is opt-in via a two-call staging protocol within a transaction: + * - stageChange(record) -- append a change record to the pending entry + * - stageAction(action, args) -- attach an action label + * Both must be called within the same transaction for the entry to land on + * the undo stack. A transaction that stages changes without an action (or + * vice versa, with no staged changes) is silently discarded at commit. + * DataObject.save() calls stageChange unconditionally for non-isNew saves; + * stageAction is opt-in via save({ undoAction, undoActionArgs }) or by an + * outer caller invoking Zotero.UndoHistory.stageAction() directly inside + * the transaction. + */ +Zotero.UndoHistory = { + _undoStack: [], + _redoStack: [], + _pendingEntry: null, + _maxSteps: 100, + + init() { + this._maxSteps = Zotero.Prefs.get('undoHistory.steps') || 100; + this.clear(); + }, + + clear() { + this._undoStack = []; + this._redoStack = []; + this._pendingEntry = null; + }, + + /** + * Return a window controller for cmd_undo/cmd_redo that defers to + * native text-editing controllers when they are active. + * Caller should append it to window.controllers. + * + * @param {Document} doc + * @return {Object} + */ + getController(doc) { + let self = this; + return { + supportsCommand(cmd) { + return cmd === 'cmd_undo' || cmd === 'cmd_redo'; + }, + isCommandEnabled(cmd) { + // Defer to native text-editing controllers when they can + // handle undo/redo (e.g. focused input/textarea) + if (self._hasNativeCommand(doc, cmd)) return false; + if (cmd === 'cmd_undo') return self.canUndo(); + if (cmd === 'cmd_redo') return self.canRedo(); + return false; + }, + doCommand(cmd) { + if (cmd === 'cmd_undo') self.undo(); + else if (cmd === 'cmd_redo') self.redo(); + }, + onEvent() {} + }; + }, + + canUndo() { + return this._undoStack.length > 0; + }, + + canRedo() { + return this._redoStack.length > 0; + }, + + /** + * Check whether the focused element has a native controller (e.g. + * text-editing) that supports undo/redo, meaning UndoHistory should defer. + * Checks the focused element's own controllers directly to avoid + * re-entrancy with the command dispatcher. + * + * @param {Document} doc + * @return {Boolean} + */ + hasNativeUndo(doc) { + return this._hasNativeCommand(doc, 'cmd_undo'); + }, + + hasNativeRedo(doc) { + return this._hasNativeCommand(doc, 'cmd_redo'); + }, + + _hasNativeCommand(doc, cmd) { + // If focus is in a child window (e.g. note-editor or reader iframe), + // it handles its own undo/redo internally + let focusedWindow = doc.commandDispatcher.focusedWindow; + if (focusedWindow && focusedWindow !== doc.defaultView) { + return true; + } + let el = doc.commandDispatcher.focusedElement; + if (!el) return false; + // Iframes (note-editor, reader) handle their own undo/redo + // internally but don't expose XUL controllers for it + if (el.tagName === 'iframe' || el.tagName === 'IFRAME') return true; + let controllers; + try { + controllers = el.controllers; + } + catch { + return false; + } + if (!controllers) return false; + for (let i = 0; i < controllers.getControllerCount(); i++) { + let ctrl = controllers.getControllerAt(i); + if (ctrl.supportsCommand(cmd)) { + return true; + } + } + return false; + }, + + /** + * Undo the most recent change entry + * + * @return {Promise} -- true if an entry was undone + */ + async undo() { + let entry = this._undoStack.pop(); + if (!entry) return false; + try { + await Zotero.DB.executeTransaction(async () => { + for (let change of entry.changes) { + let obj = this._getObject(change); + if (!obj) continue; + // Apply itemTypeID first so type-specific fields + // can be restored on the correct item type + if (change.fields.itemTypeID) { + this._applyFieldValue(obj, 'itemTypeID', change.fields.itemTypeID.old); + } + for (let [field, { old }] of Object.entries(change.fields)) { + if (field === 'itemTypeID') continue; + this._applyFieldValue(obj, field, old); + } + await obj.save({ skipSelect: true }); + } + }); + this._redoStack.push(entry); + } + catch (e) { + Zotero.logError('UndoHistory: undo failed: ' + e); + // Entry is lost -- don't push to redo + } + return true; + }, + + /** + * Redo the most recently undone entry + * + * @return {Promise} -- true if an entry was redone + */ + async redo() { + let entry = this._redoStack.pop(); + if (!entry) return false; + try { + await Zotero.DB.executeTransaction(async () => { + for (let change of entry.changes) { + let obj = this._getObject(change); + if (!obj) continue; + // Apply itemTypeID first so setType() handles + // field migration before individual fields are set + if (change.fields.itemTypeID) { + this._applyFieldValue(obj, 'itemTypeID', change.fields.itemTypeID.new); + } + for (let [field, values] of Object.entries(change.fields)) { + if (field === 'itemTypeID') continue; + this._applyFieldValue(obj, field, values.new); + } + await obj.save({ skipSelect: true }); + } + }); + this._undoStack.push(entry); + } + catch (e) { + Zotero.logError('UndoHistory: redo failed: ' + e); + // Entry is lost -- don't push to undo + } + return true; + }, + + // -- Transaction lifecycle callbacks -- + + _onTransactionBegin(_id) { + this._pendingEntry = null; + }, + + _onTransactionCommit(_id) { + // Only push entries that staged both changes and an action; + // anything else (orphan captures, action without changes) is dropped + if (this._pendingEntry && this._pendingEntry.changes.length && this._pendingEntry.action) { + this._undoStack.push(this._pendingEntry); + this._redoStack = []; + if (this._undoStack.length > this._maxSteps) { + this._undoStack.splice(0, this._undoStack.length - this._maxSteps); + } + } + this._pendingEntry = null; + }, + + _onTransactionRollback(_id) { + this._pendingEntry = null; + }, + + /** + * Stage an action label on the pending entry. Must be called inside a + * transaction. Together with one or more stageChange() calls in the same + * transaction, this is what makes the staged changes land on the undo + * stack at commit -- a transaction that doesn't call stageAction has its + * staged changes silently discarded. + * + * If called more than once in the same transaction, the last call wins. + * + * @param {String} action -- Fluent message ID (e.g. 'undo-action-add-tag') + * @param {Object} [actionArgs] -- Fluent message arguments (e.g. { count: 3 }) + */ + stageAction(action, actionArgs) { + Zotero.DB.requireTransaction(); + if (!this._pendingEntry) { + this._pendingEntry = { changes: [], action: null, actionArgs: null }; + } + this._pendingEntry.action = action; + this._pendingEntry.actionArgs = actionArgs || null; + }, + + /** + * Get the action description for the top of the undo stack + * + * @return {{ action: String, actionArgs: Object }|null} + */ + getUndoAction() { + let entry = this._undoStack[this._undoStack.length - 1]; + if (!entry || !entry.action) return null; + return { action: entry.action, actionArgs: entry.actionArgs }; + }, + + /** + * Get the action description for the top of the redo stack + * + * @return {{ action: String, actionArgs: Object }|null} + */ + getRedoAction() { + let entry = this._redoStack[this._redoStack.length - 1]; + if (!entry || !entry.action) return null; + return { action: entry.action, actionArgs: entry.actionArgs }; + }, + + /** + * Stage a change record. Must be called inside a transaction; without + * a matching stageAction() in the same transaction, the record is + * discarded at commit. + * + * Records for the same (objectType, id) are coalesced field-by-field: + * first-write-wins for `old`, last-write-wins for `new`. This lets a + * loop that saves the same object multiple times produce one composite + * record spanning the whole transaction. + * + * @param {Object} changeRecord + */ + stageChange(changeRecord) { + Zotero.DB.requireTransaction(); + if (!this._pendingEntry) { + this._pendingEntry = { changes: [], action: null, actionArgs: null }; + } + let existing = this._pendingEntry.changes.find(c => + c.objectType === changeRecord.objectType && c.id === changeRecord.id); + if (existing) { + for (let [field, vals] of Object.entries(changeRecord.fields)) { + if (existing.fields[field]) { + existing.fields[field].new = vals.new; + } + else { + existing.fields[field] = vals; + } + } + } + else { + this._pendingEntry.changes.push(changeRecord); + } + }, + + /** + * Resolve a change record to a live DataObject + * + * @param {Object} change + * @return {Zotero.DataObject|null} + */ + _getObject(change) { + let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(change.objectType); + return objectsClass ? objectsClass.get(change.id) : null; + }, + + /** + * Apply a value to the appropriate setter on an object + * + * @param {Zotero.DataObject} obj + * @param {String} field + * @param {*} value + */ + _applyFieldValue(obj, field, value) { + if (field === 'deleted') { + obj.deleted = value; + } + else if (field === 'name') { + obj.name = value; + } + else if (field === 'parentKey') { + // parentID setter routes through _setParentKey, which marks + // `parentKey` in _previousData (not parentID) + obj.parentKey = value; + } + else if (field === 'collections') { + obj.setCollections(value); + } + else if (field === 'tags') { + obj.setTags(value); + } + else if (field === 'note') { + obj.setNote(value); + } + else if (field === 'creators') { + // value is an object mapping orderIndex -> creator data (or empty object) + let maxIndex = -1; + for (let idx of Object.keys(value)) { + let i = parseInt(idx); + let creatorData = value[i]; + obj.setCreator(i, creatorData); + if (i > maxIndex) maxIndex = i; + } + // Remove any creators beyond the restored set + while (obj.hasCreatorAt(maxIndex + 1)) { + obj.removeCreator(maxIndex + 1); + } + } + else if (field === 'relations') { + // value is a flat array of [predicate, object] pairs + let relObj = {}; + for (let [predicate, object] of value) { + if (!relObj[predicate]) relObj[predicate] = []; + relObj[predicate].push(object); + } + obj.setRelations(relObj); + } + else if (field === 'itemTypeID') { + obj.setType(value); + } + else if (obj instanceof Zotero.Item) { + obj.setField(field, value); + } + else { + obj[field] = value; + } + } +}; diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js index 26b4d84a86..9e56721b15 100644 --- a/chrome/content/zotero/xpcom/zotero.js +++ b/chrome/content/zotero/xpcom/zotero.js @@ -540,6 +540,12 @@ const { CommandLineOptions } = ChromeUtils.importESModule("chrome://zotero/conte Zotero.DB.addCallback('begin', id => Zotero.Notifier.begin(id)); Zotero.DB.addCallback('commit', id => Zotero.Notifier.commit(null, id)); Zotero.DB.addCallback('rollback', id => Zotero.Notifier.reset(id)); + + // Initialize undo history and add its callbacks to the DB layer + Zotero.UndoHistory.init(); + Zotero.DB.addCallback('begin', id => Zotero.UndoHistory._onTransactionBegin(id)); + Zotero.DB.addCallback('commit', id => Zotero.UndoHistory._onTransactionCommit(id)); + Zotero.DB.addCallback('rollback', id => Zotero.UndoHistory._onTransactionRollback(id)); try { // Require >=2.1b3 database to ensure proper locking diff --git a/chrome/content/zotero/zotero.mjs b/chrome/content/zotero/zotero.mjs index c8dc7cefdc..d273aca0fb 100644 --- a/chrome/content/zotero/zotero.mjs +++ b/chrome/content/zotero/zotero.mjs @@ -112,6 +112,7 @@ const xpcomFilesLocal = [ 'locateManager', 'mime', 'notifier', + 'undoHistory', 'fileHandlers', 'plugins', 'pluginAPI/menuManager', diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index dacc823e86..027d0feef0 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -106,7 +106,11 @@ var ZoteroPane = new function () { } _loaded = true; - + + // Register a window controller for global undo/redo. Appending (rather + // than inserting at 0) ensures text-editing controllers take priority. + window.controllers.appendController(Zotero.UndoHistory.getController(document)); + var zp = document.getElementById('zotero-pane'); Zotero.UIProperties.registerRoot(zp); zp.addEventListener('UIPropertiesChanged', () => { @@ -1417,11 +1421,10 @@ var ZoteroPane = new function () { for (var i in data) { item.setField(i, data[i]); } - itemID = await item.save(); - if (collectionTreeRow && collectionTreeRow.isCollection()) { - await collectionTreeRow.ref.addItem(itemID); + item.addToCollection(collectionTreeRow.ref.id); } + itemID = await item.save(); }); // Expand the item pane if it's closed @@ -2338,6 +2341,7 @@ var ZoteroPane = new function () { skipDateModifiedUpdate: true }; await Zotero.DB.executeTransaction(async () => { + Zotero.UndoHistory.stageAction('undo-action-add-related'); for (let index1 = 0; index1 < selectedItems.length; index1++) { for (let index2 = index1 + 1; index2 < selectedItems.length; index2++) { let item1 = selectedItems[index1]; @@ -2461,6 +2465,18 @@ var ZoteroPane = new function () { let isSelected = object => selectedObjects.includes(object); await Zotero.DB.executeTransaction(async () => { + let undoAction; + if (selectedObjects.every(o => o instanceof Zotero.Item)) { + undoAction = 'undo-action-restore-items'; + } + else if (selectedObjects.every(o => o instanceof Zotero.Collection)) { + undoAction = 'undo-action-restore-collection'; + } + else { + undoAction = 'undo-action-restore-objects'; + } + Zotero.UndoHistory.stageAction(undoAction, { count: selectedObjects.length }); + for (let row = 0; row < this.itemsView.rowCount; row++) { // Only look at top-level items if (this.itemsView.getLevel(row) !== 0) { @@ -2622,7 +2638,7 @@ var ZoteroPane = new function () { selected.parentID = target.id; } - await selected.saveTx(); + await selected.saveTx({ undoAction: 'undo-action-move-collection' }); }; // Copy selected collection into another collection or library. @@ -4406,8 +4422,14 @@ var ZoteroPane = new function () { collection = Zotero.Collections.get(id); } - await Zotero.DB.executeTransaction( - () => collection.addItems(items.map(item => item.id))); + let ids = items.map(item => item.id); + await Zotero.DB.executeTransaction(async () => { + Zotero.UndoHistory.stageAction( + 'undo-action-add-to-collection', + { count: ids.length } + ); + await collection.addItems(ids); + }); }; @@ -5305,6 +5327,11 @@ var ZoteroPane = new function () { // If "Convert to Standalone Attachment" is selected, make all attachments top-level items if (shouldConvertToStandaloneAttachment) { await Zotero.DB.executeTransaction(async () => { + Zotero.UndoHistory.stageAction( + 'undo-action-convert-to-standalone', + { count: selectedItems.length } + ); + for (let item of selectedItems) { let parent = Zotero.Items.get(item.parentID); if (parent) { @@ -5327,6 +5354,11 @@ var ZoteroPane = new function () { if (!newParentItem.length) return; await Zotero.DB.executeTransaction(async () => { + Zotero.UndoHistory.stageAction( + 'undo-action-change-parent-item', + { count: selectedItems.length } + ); + for (let item of selectedItems) { item.parentID = newParentItem[0].id; await item.save({ skipSelect: true }); @@ -6388,12 +6420,13 @@ var ZoteroPane = new function () { return []; })); await Zotero.DB.executeTransaction(async () => { + Zotero.UndoHistory.stageAction('undo-action-normalize-attachment-titles'); for (let attachment of attachments) { if (attachment.getField('title').replace(/\.[^.]+$/, '') !== attachment.attachmentFilename.replace(/\.[^.]+$/, '')) { Zotero.debug(`Skipping attachment with modified title: ${attachment.getField('title')}`); continue; } - + let forceFirstOfType = !!attachment.parentItemID && await attachment.parentItem.getBestAttachment() === attachment; attachment.setAutoAttachmentTitle({ forceFirstOfType }); diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl index e951c8ab10..088b11b29c 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/chrome/locale/en-US/zotero/zotero.ftl @@ -915,3 +915,88 @@ item-pane-batch-editing-header = { $count -> [one] Editing { $count } item *[other] Editing { $count } items } + +undo-action-edit-metadata = { $count -> + [one] Edit Metadata + *[other] Edit Metadata ({ $count } items) +} +undo-action-edit-field = { $count -> + [one] Edit { $field } + *[other] Edit { $field } ({ $count } items) +} +undo-action-normalize-attachment-titles = Normalize Attachment Title +undo-action-trash = { $count -> + [one] Trash Item + *[other] Trash { $count } Items +} +undo-action-restore-items = { $count -> + [one] Restore Item + *[other] Restore { $count } Items +} +undo-action-trash-collection = { $count -> + [one] Trash Collection + *[other] Trash { $count } Collections +} +undo-action-trash-search = { $count -> + [one] Trash Saved Search + *[other] Trash { $count } Saved Searches +} +undo-action-restore-collection = { $count -> + [one] Restore Collection + *[other] Restore { $count } Collections +} +undo-action-restore-objects = { $count -> + [one] Restore Object + *[other] Restore { $count } Objects +} +undo-action-add-to-collection = { $count -> + [one] Add to Collection + *[other] Add { $count } Items to Collection +} +undo-action-remove-from-collection = { $count -> + [one] Remove from Collection + *[other] Remove { $count } Items from Collection +} +undo-action-move-to-collection = { $count -> + [one] Move to Collection + *[other] Move { $count } Items to Collection +} +undo-action-rename-collection = Rename Collection +undo-action-move-collection = Move Collection +undo-action-add-tag = { $count -> + [one] Add Tag + *[other] Add Tag to { $count } Items +} +undo-action-change-tag = Change Tag +undo-action-split-tag = Split Tag +undo-action-remove-tag = { $count -> + [one] Remove Tag + *[other] Remove Tag from { $count } Items +} +undo-action-remove-tags-from-item = { $count -> + [one] Remove Tag + *[other] Remove { $count } Tags +} +undo-action-remove-all-tags = Remove All Tags +undo-action-edit-note = Edit Note +undo-action-add-creator = Add Creator +undo-action-remove-creator = Remove Creator +undo-action-edit-creator = Edit Creator +undo-action-reorder-creator = Reorder Creator +undo-action-change-type = Change Item Type +undo-action-change-parent-item = { $count -> + [one] Change Parent Item + *[other] Change Parent for { $count } Items +} +undo-action-convert-to-standalone = { $count -> + [one] Convert to Standalone + *[other] Convert { $count } Items to Standalone +} +undo-action-add-related = Add Related +undo-action-remove-related = Remove Related +undo-action-merge-items = { $count -> + [one] Merge Item + *[other] Merge { $count } Items +} +menu-edit-undo-action = Undo { $action } +menu-edit-redo-action = Redo { $action } diff --git a/defaults/preferences/zotero.js b/defaults/preferences/zotero.js index 054b7bfd43..5e95f6c20d 100644 --- a/defaults/preferences/zotero.js +++ b/defaults/preferences/zotero.js @@ -4,6 +4,7 @@ // http://www.zotero.org/documentation/hidden_prefs pref("extensions.zotero.firstRun2", true); +pref("extensions.zotero.undoHistory.steps", 100); pref("extensions.zotero.saveRelativeAttachmentPath", false); pref("extensions.zotero.baseAttachmentPath", ""); diff --git a/test/tests/itemPaneTest.js b/test/tests/itemPaneTest.js index f3a9fea2d0..aeae4d3e68 100644 --- a/test/tests/itemPaneTest.js +++ b/test/tests/itemPaneTest.js @@ -3050,6 +3050,82 @@ describe("Item pane", function () { await group.eraseTx(); }); + it("should undo and redo a batch field edit", async function () { + let item1 = await createDataObject('item', { itemType: 'journalArticle' }); + item1.setField('publicationTitle', 'Journal Alpha'); + await item1.saveTx(); + + let item2 = await createDataObject('item', { itemType: 'journalArticle' }); + item2.setField('publicationTitle', 'Journal Beta'); + await item2.saveTx(); + + let item3 = await createDataObject('item', { itemType: 'journalArticle' }); + item3.setField('publicationTitle', 'Journal Gamma'); + await item3.saveTx(); + + await ZoteroPane.selectItems([item1.id, item2.id, item3.id]); + Zotero.UndoHistory.clear(); + + let itemPane = win.ZoteroPane.itemPane; + let itemDetails = ZoteroPane.itemPane._itemDetails; + + let batchEditEnableBtn = itemPane.querySelector('button[label="Enter Batch Edit Mode"]'); + batchEditEnableBtn.click(); + await itemDetails._renderPromise; + + let itemBox = itemPane.querySelector('#zotero-editpane-info-box'); + let pubTitleField = itemBox.querySelector('editable-text[fieldname="publicationTitle"]'); + assert.ok(pubTitleField, "publicationTitle field should exist"); + + assert.equal(pubTitleField.value, '', "field value should be empty before edit"); + assert.equal(pubTitleField.placeholder, 'Multiple\u2026', "field should show Multiple placeholder before edit"); + + pubTitleField._ignoredWindowInactiveBlur = false; + await activateZoteroPane(); + await Zotero.Promise.delay(50); + pubTitleField.focus(); + + // Options sorted alphabetically: Alpha, Beta, Gamma + "no value" option + await waitForCallback(() => pubTitleField.ref.mController.matchCount === 4, 100, 500); + // Select "Journal Alpha" from autocomplete (first entry) + let modifyPromise = waitForItemEvent('modify'); + pubTitleField.ref.dispatchEvent(new KeyboardEvent( + 'keydown', { key: "ArrowDown", code: 'ArrowDown', keyCode: KeyboardEvent.DOM_VK_DOWN, bubbles: true } + )); + await Zotero.Promise.delay(50); + pubTitleField.ref.dispatchEvent(new KeyboardEvent( + 'keydown', { key: "Enter", code: "Enter", keyCode: KeyboardEvent.DOM_VK_RETURN, bubbles: true } + )); + await modifyPromise; + // waitForItemEvent resolves during Notifier.commit, but UndoHistory's + // commit callback runs after -- wait a tick for it to complete. + await Zotero.Promise.delay(0); + + assert.equal(item1.getField('publicationTitle'), 'Journal Alpha'); + assert.equal(item2.getField('publicationTitle'), 'Journal Alpha'); + assert.equal(item3.getField('publicationTitle'), 'Journal Alpha'); + assert.isTrue(Zotero.UndoHistory.canUndo(), "should be able to undo"); + + // Undo should revert all items + await Zotero.UndoHistory.undo(); + assert.equal(item1.getField('publicationTitle'), 'Journal Alpha', + "item1 should be unchanged (already had the selected value)"); + assert.equal(item2.getField('publicationTitle'), 'Journal Beta', + "item2 should revert to original"); + assert.equal(item3.getField('publicationTitle'), 'Journal Gamma', + "item3 should revert to original"); + + // Re-query since render() rebuilds the DOM + pubTitleField = itemBox.querySelector('editable-text[fieldname="publicationTitle"]'); + assert.equal(pubTitleField.value, '', "field value should be empty after undo"); + assert.equal(pubTitleField.placeholder, 'Multiple\u2026', "field should show Multiple placeholder after undo"); + + // Redo should re-apply to all items + await Zotero.UndoHistory.redo(); + assert.equal(item1.getField('publicationTitle'), 'Journal Alpha'); + assert.equal(item2.getField('publicationTitle'), 'Journal Alpha'); + assert.equal(item3.getField('publicationTitle'), 'Journal Alpha'); + }); it("should transform title case for all items in batch edit mode", async function () { let titleCaseTitle = "The Great Gatsby"; let sentenceCaseTitle = "to kill a mockingbird"; @@ -3113,6 +3189,66 @@ describe("Item pane", function () { assert.equal(item1.getField('title'), "The Great Gatsby", "item1 should remain in title case"); assert.equal(item2.getField('title'), "To Kill a Mockingbird", "item2 should be transformed to title case"); }); + + it("should show options button and transform case when primary item field is empty", async function () { + // Item 1 has no seriesTitle, items 2 and 3 do + let item1 = await createDataObject('item', { itemType: 'journalArticle' }); + await item1.saveTx(); + + let item2 = await createDataObject('item', { itemType: 'journalArticle' }); + item2.setField('seriesTitle', 'advances in neural information processing'); + await item2.saveTx(); + + let item3 = await createDataObject('item', { itemType: 'journalArticle' }); + item3.setField('seriesTitle', 'proceedings of the ACM conference'); + await item3.saveTx(); + + await ZoteroPane.selectItems([item1.id, item2.id, item3.id]); + + let itemPane = win.ZoteroPane.itemPane; + let itemDetails = ZoteroPane.itemPane._itemDetails; + + let batchEditEnableBtn = itemPane.querySelector('button[label="Enter Batch Edit Mode"]'); + batchEditEnableBtn.click(); + await itemDetails._renderPromise; + + let itemBox = itemPane.querySelector('#zotero-editpane-info-box'); + let optionsButton = itemBox.querySelector('#itembox-field-seriesTitle-options'); + assert.ok(optionsButton, "options button should exist for seriesTitle"); + assert.isFalse(optionsButton.hidden, "options button should be visible when extra items have values"); + + // Open the context menu via the options button + let menuPromise = new Promise((resolve) => { + let observer = new MutationObserver((mutations) => { + for (let mutation of mutations) { + for (let node of mutation.addedNodes) { + if (node.tagName === 'menupopup') { + observer.disconnect(); + resolve(node); + } + } + } + }); + observer.observe(itemBox.querySelector('#info-box > popupset'), { childList: true }); + }); + + optionsButton.click(); + let menupopup = await menuPromise; + + let titleCaseMenuItem = Array.from(menupopup.querySelectorAll('menuitem')) + .find(mi => mi.getAttribute('label') === Zotero.getString('zotero.item.textTransform.titlecase')); + assert.ok(titleCaseMenuItem, "title case menu item should exist"); + + let modifyPromise = waitForItemEvent('modify'); + titleCaseMenuItem.click(); + await modifyPromise; + + assert.equal(item1.getField('seriesTitle'), '', "item1 should remain empty"); + assert.equal(item2.getField('seriesTitle'), 'Advances in Neural Information Processing', + "item2 should be transformed to title case"); + assert.equal(item3.getField('seriesTitle'), 'Proceedings of the ACM Conference', + "item3 should be transformed to title case"); + }); }); it("should not focus read-only fields with multiple values", async function () { diff --git a/test/tests/undoHistoryTest.js b/test/tests/undoHistoryTest.js new file mode 100644 index 0000000000..a811e3a23a --- /dev/null +++ b/test/tests/undoHistoryTest.js @@ -0,0 +1,1049 @@ +describe("Zotero.UndoHistory", function () { + beforeEach(function () { + Zotero.UndoHistory.clear(); + }); + + describe("collection name edit", function () { + it("should undo and redo a collection name change", async function () { + let collection = await createDataObject('collection', { name: 'Original' }); + + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + assert.equal(collection.name, 'Modified'); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + await Zotero.UndoHistory.undo(); + assert.equal(collection.name, 'Original'); + assert.isTrue(Zotero.UndoHistory.canRedo()); + + await Zotero.UndoHistory.redo(); + assert.equal(collection.name, 'Modified'); + }); + }); + + describe("trashing a collection", function () { + it("should undo and redo trashing a collection", async function () { + let collection = await createDataObject('collection'); + + collection.deleted = true; + await collection.saveTx({ + undoAction: 'undo-action-trash-collection', + undoActionArgs: { count: 1 } + }); + assert.isTrue(collection.deleted); + + await Zotero.UndoHistory.undo(); + assert.isFalse(collection.deleted); + + await Zotero.UndoHistory.redo(); + assert.isTrue(collection.deleted); + }); + + it("should undo and redo trashing a collection with descendent sub-collections", async function () { + let parent = await createDataObject('collection', { name: 'Parent' }); + let child = await createDataObject('collection', { name: 'Child', parentID: parent.id }); + Zotero.UndoHistory.clear(); + + parent.deleted = true; + await parent.saveTx({ + undoAction: 'undo-action-trash-collection', + undoActionArgs: { count: 1 } + }); + assert.isTrue(parent.deleted); + assert.isTrue(child.deleted); + + await Zotero.UndoHistory.undo(); + assert.isFalse(parent.deleted); + assert.isFalse(child.deleted); + + await Zotero.UndoHistory.redo(); + assert.isTrue(parent.deleted); + assert.isTrue(child.deleted); + }); + }); + + describe("trashing items via Items.trashTx", function () { + it("should undo and redo trashing an item", async function () { + let item = await createDataObject('item', { title: 'Trash Me' }); + + await Zotero.Items.trashTx(item.id); + assert.isTrue(item.deleted); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + await Zotero.UndoHistory.undo(); + assert.isFalse(item.deleted); + + await Zotero.UndoHistory.redo(); + assert.isTrue(item.deleted); + }); + + it("should undo trashing multiple items as a single step", async function () { + let item1 = await createDataObject('item', { title: 'Item 1' }); + let item2 = await createDataObject('item', { title: 'Item 2' }); + Zotero.UndoHistory.clear(); + + await Zotero.Items.trashTx([item1.id, item2.id]); + assert.isTrue(item1.deleted); + assert.isTrue(item2.deleted); + + // Should be a single undo step + await Zotero.UndoHistory.undo(); + assert.isFalse(item1.deleted); + assert.isFalse(item2.deleted); + assert.isFalse(Zotero.UndoHistory.canUndo()); + }); + }); + + describe("item metadata field edit", function () { + it("should undo and redo a single item field change", async function () { + let item = await createDataObject('item', { title: 'Original Title' }); + + item.setField('title', 'New Title'); + await item.saveTx({ + undoAction: 'undo-action-edit-metadata', + undoActionArgs: { count: 1 } + }); + assert.equal(item.getField('title'), 'New Title'); + + await Zotero.UndoHistory.undo(); + assert.equal(item.getField('title'), 'Original Title'); + + await Zotero.UndoHistory.redo(); + assert.equal(item.getField('title'), 'New Title'); + }); + }); + + describe("batch metadata edit", function () { + it("should undo a batch edit as a single step", async function () { + let item1 = await createDataObject('item', { title: 'Title A' }); + let item2 = await createDataObject('item', { title: 'Title B' }); + Zotero.UndoHistory.clear(); + + await Zotero.DB.executeTransaction(async function () { + item1.setField('title', 'Batch Title'); + await item1.save(); + item2.setField('title', 'Batch Title'); + await item2.save(); + Zotero.UndoHistory.stageAction( + 'undo-action-edit-metadata', { count: 2 } + ); + }); + + assert.equal(item1.getField('title'), 'Batch Title'); + assert.equal(item2.getField('title'), 'Batch Title'); + + // Single undo should revert both + await Zotero.UndoHistory.undo(); + assert.equal(item1.getField('title'), 'Title A'); + assert.equal(item2.getField('title'), 'Title B'); + assert.isFalse(Zotero.UndoHistory.canUndo()); + }); + }); + + describe("opt-in capture", function () { + it("should not record a save without an undoAction", async function () { + let collection = await createDataObject('collection', { name: 'Original' }); + Zotero.UndoHistory.clear(); + + collection.name = 'Modified'; + await collection.saveTx(); + assert.equal(collection.name, 'Modified'); + assert.isFalse(Zotero.UndoHistory.canUndo()); + }); + + it("should not record a save with skipAll", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + item.setField('title', 'Modified'); + await item.saveTx({ skipAll: true }); + assert.isFalse(Zotero.UndoHistory.canUndo()); + }); + + it("should drop staged changes if stageAction is never called", async function () { + let item1 = await createDataObject('item', { title: 'A' }); + let item2 = await createDataObject('item', { title: 'B' }); + Zotero.UndoHistory.clear(); + + await Zotero.DB.executeTransaction(async function () { + item1.setField('title', 'X'); + await item1.save(); + item2.setField('title', 'Y'); + await item2.save(); + // no stageAction call + }); + + assert.isFalse(Zotero.UndoHistory.canUndo()); + }); + }); + + describe("redo stack", function () { + it("should clear redo stack on new change", async function () { + let collection = await createDataObject('collection', { name: 'V1' }); + + collection.name = 'V2'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + + await Zotero.UndoHistory.undo(); + assert.isTrue(Zotero.UndoHistory.canRedo()); + + // New change should clear the redo stack + collection.name = 'V3'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + assert.isFalse(Zotero.UndoHistory.canRedo()); + }); + }); + + describe("deleted object handling", function () { + it("should handle a deleted object gracefully during undo", async function () { + let collection = await createDataObject('collection', { name: 'Original' }); + + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + + // Permanently delete the collection + await collection.eraseTx(); + + // Undo should not throw + let result = await Zotero.UndoHistory.undo(); + assert.isTrue(result); + }); + }); + + describe("canUndo/canRedo", function () { + it("should return false when stacks are empty", function () { + assert.isFalse(Zotero.UndoHistory.canUndo()); + assert.isFalse(Zotero.UndoHistory.canRedo()); + }); + + it("should return false after undo with no redo available when nothing undone", async function () { + let result = await Zotero.UndoHistory.undo(); + assert.isFalse(result); + }); + + it("should return false after redo with nothing to redo", async function () { + let result = await Zotero.UndoHistory.redo(); + assert.isFalse(result); + }); + }); + + describe("collection membership changes", function () { + it("should undo and redo adding an item to a collection", async function () { + let collection = await createDataObject('collection'); + let item = await createDataObject('item', { title: 'Test Item' }); + Zotero.UndoHistory.clear(); + + item.setCollections([collection.id]); + await item.saveTx({ + undoAction: 'undo-action-add-to-collection', + undoActionArgs: { count: 1 } + }); + assert.include(item.getCollections(), collection.id); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + await Zotero.UndoHistory.undo(); + assert.notInclude(item.getCollections(), collection.id); + assert.lengthOf(item.getCollections(), 0); + + await Zotero.UndoHistory.redo(); + assert.include(item.getCollections(), collection.id); + }); + + it("should undo and redo removing an item from a collection", async function () { + let collection = await createDataObject('collection'); + let item = await createDataObject('item', { + title: 'Test Item', + collections: [collection.id] + }); + assert.include(item.getCollections(), collection.id); + Zotero.UndoHistory.clear(); + + item.setCollections([]); + await item.saveTx({ + undoAction: 'undo-action-remove-from-collection', + undoActionArgs: { count: 1 } + }); + assert.lengthOf(item.getCollections(), 0); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + await Zotero.UndoHistory.undo(); + assert.include(item.getCollections(), collection.id); + + await Zotero.UndoHistory.redo(); + assert.lengthOf(item.getCollections(), 0); + }); + }); + + describe("parent change", function () { + it("should undo and redo unparenting a child item", async function () { + let parent = await createDataObject('item', { title: 'Parent' }); + let child = new Zotero.Item('note'); + child.parentID = parent.id; + child.setNote('Child note'); + await child.saveTx(); + assert.equal(child.parentID, parent.id); + Zotero.UndoHistory.clear(); + + child.parentID = false; + await child.saveTx({ + undoAction: 'undo-action-convert-to-standalone-attachment', + undoActionArgs: { count: 1 } + }); + assert.isFalse(!!child.parentID); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + await Zotero.UndoHistory.undo(); + assert.equal(child.parentID, parent.id); + + await Zotero.UndoHistory.redo(); + assert.isFalse(!!child.parentID); + }); + }); + + describe("note edit", function () { + it("should undo and redo a note text change", async function () { + let item = new Zotero.Item('note'); + item.setNote('Original note'); + await item.saveTx(); + Zotero.UndoHistory.clear(); + + item.setNote('Modified note'); + await item.saveTx({ undoAction: 'undo-action-edit-note' }); + assert.equal(item.getNote(), 'Modified note'); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + let action = Zotero.UndoHistory.getUndoAction(); + assert.equal(action.action, 'undo-action-edit-note'); + + await Zotero.UndoHistory.undo(); + assert.equal(item.getNote(), 'Original note'); + assert.isTrue(Zotero.UndoHistory.canRedo()); + + await Zotero.UndoHistory.redo(); + assert.equal(item.getNote(), 'Modified note'); + }); + }); + + describe("action tracking", function () { + describe("explicit action", function () { + it("should use undoAction option from saveTx", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + item.setField('title', 'Changed'); + await item.saveTx({ undoAction: 'undo-action-change-type' }); + + let action = Zotero.UndoHistory.getUndoAction(); + assert.isNotNull(action); + assert.equal(action.action, 'undo-action-change-type'); + }); + + it("should use stageAction called inside a transaction", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + await Zotero.DB.executeTransaction(async function () { + item.setField('title', 'Changed'); + await item.save(); + Zotero.UndoHistory.stageAction( + 'undo-action-edit-metadata', { count: 1 } + ); + }); + + let action = Zotero.UndoHistory.getUndoAction(); + assert.isNotNull(action); + assert.equal(action.action, 'undo-action-edit-metadata'); + assert.deepEqual(action.actionArgs, { count: 1 }); + }); + + it("should let the last stageAction call win", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + await Zotero.DB.executeTransaction(async function () { + item.setField('title', 'Changed'); + await item.save(); + Zotero.UndoHistory.stageAction('undo-action-edit-metadata'); + Zotero.UndoHistory.stageAction('undo-action-change-type'); + }); + + let action = Zotero.UndoHistory.getUndoAction(); + assert.equal(action.action, 'undo-action-change-type'); + }); + }); + + describe("redo preservation", function () { + it("should preserve action through undo/redo cycle", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + item.setField('title', 'Changed'); + await item.saveTx({ + undoAction: 'undo-action-edit-metadata', + undoActionArgs: { count: 1 } + }); + + let undoAction = Zotero.UndoHistory.getUndoAction(); + assert.equal(undoAction.action, 'undo-action-edit-metadata'); + + await Zotero.UndoHistory.undo(); + + let redoAction = Zotero.UndoHistory.getRedoAction(); + assert.isNotNull(redoAction); + assert.equal(redoAction.action, 'undo-action-edit-metadata'); + assert.deepEqual(redoAction.actionArgs, { count: 1 }); + + await Zotero.UndoHistory.redo(); + + undoAction = Zotero.UndoHistory.getUndoAction(); + assert.isNotNull(undoAction); + assert.equal(undoAction.action, 'undo-action-edit-metadata'); + }); + }); + + describe("getUndoAction/getRedoAction", function () { + it("should return null when stacks are empty", function () { + assert.isNull(Zotero.UndoHistory.getUndoAction()); + assert.isNull(Zotero.UndoHistory.getRedoAction()); + }); + + it("should return null for redo when nothing has been undone", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + item.setField('title', 'Changed'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + + assert.isNotNull(Zotero.UndoHistory.getUndoAction()); + assert.isNull(Zotero.UndoHistory.getRedoAction()); + }); + }); + }); + + describe("creator changes", function () { + it("should undo and redo editing a creator name", async function () { + let item = await createDataObject('item'); + item.setCreator(0, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: 'John', + lastName: 'Doe', + fieldMode: 0 + }); + await item.saveTx(); + Zotero.UndoHistory.clear(); + + item.setCreator(0, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: 'John', + lastName: 'Smith', + fieldMode: 0 + }); + await item.saveTx({ undoAction: 'undo-action-edit-creator' }); + assert.equal(item.getCreator(0).lastName, 'Smith'); + + await Zotero.UndoHistory.undo(); + assert.equal(item.getCreator(0).lastName, 'Doe'); + + await Zotero.UndoHistory.redo(); + assert.equal(item.getCreator(0).lastName, 'Smith'); + }); + + it("should undo and redo adding a new creator", async function () { + let item = await createDataObject('item'); + item.setCreator(0, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: 'Jane', + lastName: 'Doe', + fieldMode: 0 + }); + await item.saveTx(); + Zotero.UndoHistory.clear(); + + item.setCreator(1, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: 'Bob', + lastName: 'Jones', + fieldMode: 0 + }); + await item.saveTx({ undoAction: 'undo-action-add-creator' }); + assert.equal(item.numCreators(), 2); + + await Zotero.UndoHistory.undo(); + assert.equal(item.numCreators(), 1); + assert.equal(item.getCreator(0).lastName, 'Doe'); + + await Zotero.UndoHistory.redo(); + assert.equal(item.numCreators(), 2); + assert.equal(item.getCreator(1).lastName, 'Jones'); + }); + + it("should undo and redo removing a creator", async function () { + let item = await createDataObject('item'); + item.setCreator(0, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: 'Jane', + lastName: 'Doe', + fieldMode: 0 + }); + item.setCreator(1, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: 'Bob', + lastName: 'Jones', + fieldMode: 0 + }); + await item.saveTx(); + Zotero.UndoHistory.clear(); + + item.removeCreator(1); + await item.saveTx({ undoAction: 'undo-action-remove-creator' }); + assert.equal(item.numCreators(), 1); + + await Zotero.UndoHistory.undo(); + assert.equal(item.numCreators(), 2); + assert.equal(item.getCreator(1).lastName, 'Jones'); + + await Zotero.UndoHistory.redo(); + assert.equal(item.numCreators(), 1); + }); + + it("should undo and redo changing creator type", async function () { + let item = await createDataObject('item'); + item.setCreator(0, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: 'John', + lastName: 'Doe', + fieldMode: 0 + }); + await item.saveTx(); + Zotero.UndoHistory.clear(); + + item.setCreator(0, { + creatorTypeID: Zotero.CreatorTypes.getID('editor'), + firstName: 'John', + lastName: 'Doe', + fieldMode: 0 + }); + await item.saveTx({ undoAction: 'undo-action-edit-creator' }); + assert.equal(item.getCreator(0).creatorTypeID, Zotero.CreatorTypes.getID('editor')); + + await Zotero.UndoHistory.undo(); + assert.equal(item.getCreator(0).creatorTypeID, Zotero.CreatorTypes.getID('author')); + + await Zotero.UndoHistory.redo(); + assert.equal(item.getCreator(0).creatorTypeID, Zotero.CreatorTypes.getID('editor')); + }); + + it("should undo and redo switching field mode", async function () { + let item = await createDataObject('item'); + item.setCreator(0, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: 'John', + lastName: 'Doe', + fieldMode: 0 + }); + await item.saveTx(); + Zotero.UndoHistory.clear(); + + item.setCreator(0, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: '', + lastName: 'John Doe', + fieldMode: 1 + }); + await item.saveTx({ undoAction: 'undo-action-edit-creator' }); + assert.equal(item.getCreator(0).fieldMode, 1); + assert.equal(item.getCreator(0).lastName, 'John Doe'); + + await Zotero.UndoHistory.undo(); + assert.equal(item.getCreator(0).fieldMode, 0); + assert.equal(item.getCreator(0).firstName, 'John'); + assert.equal(item.getCreator(0).lastName, 'Doe'); + + await Zotero.UndoHistory.redo(); + assert.equal(item.getCreator(0).fieldMode, 1); + }); + + it("should undo and redo reordering creators", async function () { + let item = await createDataObject('item'); + item.setCreator(0, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: 'First', + lastName: 'Author', + fieldMode: 0 + }); + item.setCreator(1, { + creatorTypeID: Zotero.CreatorTypes.getID('author'), + firstName: 'Second', + lastName: 'Author', + fieldMode: 0 + }); + await item.saveTx(); + Zotero.UndoHistory.clear(); + + // Swap order -- move second to first position + let creators = item.getCreators(); + item.setCreator(0, creators[1]); + item.setCreator(1, creators[0]); + await item.saveTx({ undoAction: 'undo-action-reorder-creator' }); + assert.equal(item.getCreator(0).firstName, 'Second'); + assert.equal(item.getCreator(1).firstName, 'First'); + + await Zotero.UndoHistory.undo(); + assert.equal(item.getCreator(0).firstName, 'First'); + assert.equal(item.getCreator(1).firstName, 'Second'); + + await Zotero.UndoHistory.redo(); + assert.equal(item.getCreator(0).firstName, 'Second'); + assert.equal(item.getCreator(1).firstName, 'First'); + }); + }); + + describe("item type change", function () { + it("should undo and redo a type change that loses fields", async function () { + let caseTypeID = Zotero.ItemTypes.getID('case'); + let filmTypeID = Zotero.ItemTypes.getID('film'); + + let item = await createDataObject('item', { itemType: 'case' }); + item.setField('court', 'Supreme Court'); + await item.saveTx(); + Zotero.UndoHistory.clear(); + + // Change type: Case -> Film (court is lost) + item.setType(filmTypeID); + await item.saveTx({ undoAction: 'undo-action-change-type' }); + + assert.equal(item.itemTypeID, filmTypeID); + assert.equal(item.getField('court'), ''); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + // Undo: Film -> Case, court restored + await Zotero.UndoHistory.undo(); + assert.equal(item.itemTypeID, caseTypeID); + assert.equal(item.getField('court'), 'Supreme Court'); + assert.isFalse(Zotero.UndoHistory.canUndo(), "only one undo entry should exist"); + assert.isTrue(Zotero.UndoHistory.canRedo()); + + // Redo: Case -> Film (no dialog -- goes through UndoHistory.redo()) + await Zotero.UndoHistory.redo(); + assert.equal(item.itemTypeID, filmTypeID); + assert.equal(item.getField('court'), ''); + }); + }); + + describe("related items", function () { + it("should undo and redo adding a related item", async function () { + let itemA = await createDataObject('item', { title: 'Item A' }); + let itemB = await createDataObject('item', { title: 'Item B' }); + Zotero.UndoHistory.clear(); + + await Zotero.DB.executeTransaction(async () => { + itemA.addRelatedItem(itemB); + await itemA.save({ skipDateModifiedUpdate: true }); + itemB.addRelatedItem(itemA); + await itemB.save({ skipDateModifiedUpdate: true }); + Zotero.UndoHistory.stageAction('undo-action-add-related'); + }); + + assert.include(itemA.relatedItems, itemB.key); + assert.include(itemB.relatedItems, itemA.key); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + await Zotero.UndoHistory.undo(); + assert.notInclude(itemA.relatedItems, itemB.key); + assert.notInclude(itemB.relatedItems, itemA.key); + + await Zotero.UndoHistory.redo(); + assert.include(itemA.relatedItems, itemB.key); + assert.include(itemB.relatedItems, itemA.key); + }); + + it("should undo and redo removing a related item", async function () { + let itemA = await createDataObject('item', { title: 'Item A' }); + let itemB = await createDataObject('item', { title: 'Item B' }); + // Establish the relation + await Zotero.DB.executeTransaction(async () => { + itemA.addRelatedItem(itemB); + await itemA.save({ skipDateModifiedUpdate: true }); + itemB.addRelatedItem(itemA); + await itemB.save({ skipDateModifiedUpdate: true }); + }); + Zotero.UndoHistory.clear(); + + // Remove the relation + await Zotero.DB.executeTransaction(async () => { + itemA.removeRelatedItem(itemB); + await itemA.save({ skipDateModifiedUpdate: true }); + itemB.removeRelatedItem(itemA); + await itemB.save({ skipDateModifiedUpdate: true }); + Zotero.UndoHistory.stageAction('undo-action-remove-related'); + }); + + assert.notInclude(itemA.relatedItems, itemB.key); + assert.notInclude(itemB.relatedItems, itemA.key); + + await Zotero.UndoHistory.undo(); + assert.include(itemA.relatedItems, itemB.key); + assert.include(itemB.relatedItems, itemA.key); + + await Zotero.UndoHistory.redo(); + assert.notInclude(itemA.relatedItems, itemB.key); + assert.notInclude(itemB.relatedItems, itemA.key); + }); + + it("should undo adding several related items in one transaction", async function () { + let subject = await createDataObject('item', { title: 'Subject' }); + let relA = await createDataObject('item', { title: 'Rel A' }); + let relB = await createDataObject('item', { title: 'Rel B' }); + let relC = await createDataObject('item', { title: 'Rel C' }); + Zotero.UndoHistory.clear(); + + await Zotero.DB.executeTransaction(async () => { + Zotero.UndoHistory.stageAction('undo-action-add-related'); + for (let rel of [relA, relB, relC]) { + subject.addRelatedItem(rel); + await subject.save({ skipDateModifiedUpdate: true }); + rel.addRelatedItem(subject); + await rel.save({ skipDateModifiedUpdate: true }); + } + }); + + assert.include(subject.relatedItems, relA.key); + assert.include(subject.relatedItems, relB.key); + assert.include(subject.relatedItems, relC.key); + + await Zotero.UndoHistory.undo(); + assert.notInclude(subject.relatedItems, relA.key); + assert.notInclude(subject.relatedItems, relB.key); + assert.notInclude(subject.relatedItems, relC.key); + assert.notInclude(relA.relatedItems, subject.key); + assert.notInclude(relB.relatedItems, subject.key); + assert.notInclude(relC.relatedItems, subject.key); + + await Zotero.UndoHistory.redo(); + assert.include(subject.relatedItems, relA.key); + assert.include(subject.relatedItems, relB.key); + assert.include(subject.relatedItems, relC.key); + }); + }); + + describe("staging guards", function () { + it("should throw if stageChange is called outside a transaction", function () { + assert.throws( + () => Zotero.UndoHistory.stageChange({ + objectType: 'item', + id: 1, + libraryID: 1, + key: 'AAAAAAAA', + fields: {} + }), + /transaction/i + ); + }); + + it("should throw if stageAction is called outside a transaction", function () { + assert.throws( + () => Zotero.UndoHistory.stageAction('undo-action-edit-metadata'), + /transaction/i + ); + }); + }); + + describe("sync interaction", function () { + var apiKey = Zotero.Utilities.randomString(24); + var baseURL = "http://local.zotero/"; + var server; + + beforeEach(async function () { + await resetData(); + Zotero.HTTP.mock = sinon.FakeXMLHttpRequest; + server = sinon.fakeServer.create(); + server.autoRespond = true; + await Zotero.Users.setCurrentUserID(1); + await Zotero.Users.setCurrentUsername("A"); + Zotero.UndoHistory.clear(); + + // Minimal pre-engine stubs (mirrors syncRunnerTest.js) + server.respondWith("GET", baseURL + "keys/current", [200, + { "Content-Type": "application/json" }, + JSON.stringify({ + key: apiKey, + userID: 1, + username: "A", + access: { + user: { library: true, files: true, notes: true, write: true }, + groups: { all: { library: true, write: true } } + } + })]); + server.respondWith("GET", baseURL + "users/1/groups?format=versions", + [200, { "Content-Type": "application/json" }, "{}"]); + }); + + afterEach(function () { + Zotero.HTTP.mock = null; + }); + + function setNoRemoteChangesResponses(lastLibraryVersion) { + // Server reports library unmodified for every endpoint sync hits. + let headers = { "Last-Modified-Version": lastLibraryVersion }; + let target = "users/1"; + let endpoints = [ + `${target}/settings?since=${lastLibraryVersion}`, + `${target}/collections?format=versions&since=${lastLibraryVersion}`, + `${target}/searches?format=versions&since=${lastLibraryVersion}`, + `${target}/items/top?format=versions&since=${lastLibraryVersion}&includeTrashed=1`, + `${target}/items?format=versions&since=${lastLibraryVersion}&includeTrashed=1`, + `${target}/deleted?since=${lastLibraryVersion}` + ]; + for (let url of endpoints) { + server.respondWith("GET", baseURL + url, [ + 304, headers, "" + ]); + } + // Full-text sync probes this regardless of the data-sync result + server.respondWith("GET", baseURL + `${target}/fulltext?format=versions`, + [200, headers, "{}"]); + } + + it("preserves the undo stack when sync has no remote changes to apply", async function () { + let library = Zotero.Libraries.userLibrary; + let lastLibraryVersion = 5; + library.libraryVersion = library.storageVersion = lastLibraryVersion; + await library.saveTx(); + + let item = await createDataObject('item', { title: 'Before' }); + Zotero.UndoHistory.clear(); + item.setField('title', 'After'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + assert.isTrue(Zotero.UndoHistory.canUndo(), + "sanity: an undo entry exists before sync"); + + // Re-mark synced so the upload phase is a no-op + await Zotero.Sync.Data.Local.markObjectAsSynced(item); + assert.isTrue(Zotero.UndoHistory.canUndo(), + "sanity: markObjectAsSynced did not clear the undo stack"); + + setNoRemoteChangesResponses(lastLibraryVersion); + + let runner = new Zotero.Sync.Runner_Module({ baseURL, apiKey }); + await runner._sync({ + libraries: [library.libraryID], + onError: e => { throw e; } + }); + + assert.isTrue(Zotero.UndoHistory.canUndo(), + "undo stack should survive a sync with no remote changes"); + }); + + it("clears the undo stack when _saveObjectFromJSON applies a remote object", async function () { + let library = Zotero.Libraries.userLibrary; + let lastLibraryVersion = 5; + let newLibraryVersion = 6; + library.libraryVersion = library.storageVersion = lastLibraryVersion; + await library.saveTx(); + + let item = await createDataObject('item', { title: 'Before' }); + item.version = lastLibraryVersion; + await Zotero.Sync.Data.Local.markObjectAsSynced(item); + let itemKey = item.key; + + let other = await createDataObject('item', { title: 'Other-before' }); + Zotero.UndoHistory.clear(); + other.setField('title', 'Other-after'); + await other.saveTx({ undoAction: 'undo-action-edit-metadata' }); + await Zotero.Sync.Data.Local.markObjectAsSynced(other); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + setNoRemoteChangesResponses(lastLibraryVersion); + + let libraryID = library.libraryID; + let remoteJSON = [{ + key: itemKey, + version: newLibraryVersion, + data: Object.assign({}, item.toJSON(), { + key: itemKey, + version: newLibraryVersion, + title: 'Remote-applied title' + }) + }]; + let engineStub = sinon.stub(Zotero.Sync.Data.Engine.prototype, 'start') + .callsFake(async function () { + await Zotero.Sync.Data.Local.processObjectsFromJSON( + 'item', libraryID, remoteJSON, {} + ); + }); + + try { + let runner = new Zotero.Sync.Runner_Module({ baseURL, apiKey }); + await runner._sync({ + libraries: [libraryID], + onError: e => { throw e; } + }); + } + finally { + engineStub.restore(); + } + + assert.equal(item.getField('title'), 'Remote-applied title', + "sanity: remote data was applied via _saveObjectFromJSON"); + assert.isFalse(Zotero.UndoHistory.canUndo(), + "undo stack should be cleared once _saveObjectFromJSON marks the sync"); + }); + + it("clears the undo stack when sync applies a remote deletion", async function () { + let library = Zotero.Libraries.userLibrary; + let lastLibraryVersion = 5; + let newLibraryVersion = 6; + library.libraryVersion = library.storageVersion = lastLibraryVersion; + await library.saveTx(); + + // An item that the server has deleted. Mark synced so the engine + // treats it as a clean deletion rather than a deletion conflict. + let item = await createDataObject('item', { title: 'Remote-deleted' }); + item.version = lastLibraryVersion; + await Zotero.Sync.Data.Local.markObjectAsSynced(item); + let itemKey = item.key; + + // Separate undoable edit on a different item so the stack is + // non-empty and unrelated to the deleted object. + let other = await createDataObject('item', { title: 'Other-before' }); + Zotero.UndoHistory.clear(); + other.setField('title', 'Other-after'); + await other.saveTx({ undoAction: 'undo-action-edit-metadata' }); + await Zotero.Sync.Data.Local.markObjectAsSynced(other); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + let newHeaders = { "Last-Modified-Version": newLibraryVersion }; + let url = u => server.respondWith("GET", baseURL + u, [200, newHeaders, "{}"]); + url(`users/1/settings?since=${lastLibraryVersion}`); + url(`users/1/collections?format=versions&since=${lastLibraryVersion}`); + url(`users/1/searches?format=versions&since=${lastLibraryVersion}`); + url(`users/1/items/top?format=versions&since=${lastLibraryVersion}&includeTrashed=1`); + url(`users/1/items?format=versions&since=${lastLibraryVersion}&includeTrashed=1`); + url(`users/1/fulltext?format=versions`); + + // The deletion endpoint reports our item as remotely deleted + server.respondWith("GET", + baseURL + `users/1/deleted?since=${lastLibraryVersion}`, + [200, newHeaders, JSON.stringify({ + items: [itemKey], collections: [], searches: [], tags: [], settings: [] + })]); + + let runner = new Zotero.Sync.Runner_Module({ baseURL, apiKey }); + await runner._sync({ + libraries: [library.libraryID], + onError: e => { throw e; } + }); + + assert.isFalse(Zotero.Items.exists(item.id), + "sanity: remote deletion was applied locally"); + assert.isFalse(Zotero.UndoHistory.canUndo(), + "undo stack should be cleared when sync applies a remote deletion"); + }); + + it("clears the undo stack when _restoreRestoredCollectionItems applies remote changes", async function () { + // When a collection is deleted locally but modified remotely, sync re-creates + // the collection (covered by _saveObjectFromJSON) and then separately un-trashes + // items that were trashed with the collection and re-adds them to it. Both are + // remote-driven mutations to user-visible item state and must clear the stack. + let library = Zotero.Libraries.userLibrary; + let lastLibraryVersion = 5; + library.libraryVersion = library.storageVersion = lastLibraryVersion; + await library.saveTx(); + + let collection = await createDataObject('collection', { name: 'Restored' }); + await Zotero.Sync.Data.Local.markObjectAsSynced(collection); + let collectionKey = collection.key; + + let item = await createDataObject('item', { title: 'Restored item' }); + item.deleted = true; + await item.saveTx(); + await Zotero.Sync.Data.Local.markObjectAsSynced(item); + let itemKey = item.key; + + let other = await createDataObject('item', { title: 'Other-before' }); + Zotero.UndoHistory.clear(); + other.setField('title', 'Other-after'); + await other.saveTx({ undoAction: 'undo-action-edit-metadata' }); + await Zotero.Sync.Data.Local.markObjectAsSynced(other); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + setNoRemoteChangesResponses(lastLibraryVersion); + + // _restoreRestoredCollectionItems queries top items in the restored collection + server.respondWith("GET", + baseURL + `users/1/collections/${collectionKey}/items/top?format=keys`, + [200, { "Last-Modified-Version": lastLibraryVersion }, itemKey]); + + let libraryID = library.libraryID; + let engineStub = sinon.stub(Zotero.Sync.Data.Engine.prototype, 'start') + .callsFake(async function () { + await this._restoreRestoredCollectionItems([collectionKey]); + }); + + try { + let runner = new Zotero.Sync.Runner_Module({ baseURL, apiKey }); + await runner._sync({ + libraries: [libraryID], + onError: e => { throw e; } + }); + } + finally { + engineStub.restore(); + } + + let restored = Zotero.Items.get(item.id); + assert.isFalse(restored.deleted, + "sanity: trashed item was un-trashed by the restoration"); + assert.isTrue(restored.inCollection(collection.id), + "sanity: item was re-added to the restored collection"); + assert.isFalse(Zotero.UndoHistory.canUndo(), + "undo stack should be cleared when _restoreRestoredCollectionItems mutates items"); + }); + + it("clears the undo stack across a restartSync recursion", async function () { + // Regression: the recursive _sync() call resets the flag at its + // start, so the conditional clear must happen before the restart + // branch, not after. + let library = Zotero.Libraries.userLibrary; + let lastLibraryVersion = 5; + library.libraryVersion = library.storageVersion = lastLibraryVersion; + await library.saveTx(); + + let item = await createDataObject('item', { title: 'Before' }); + Zotero.UndoHistory.clear(); + item.setField('title', 'After'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + await Zotero.Sync.Data.Local.markObjectAsSynced(item); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + setNoRemoteChangesResponses(lastLibraryVersion); + + // First engine iteration applies a remote change; the second is a no-op + let callCount = 0; + let engineStub = sinon.stub(Zotero.Sync.Data.Engine.prototype, 'start') + .callsFake(async function () { + if (callCount === 0) { + Zotero.Sync.Data.Local.markRemoteChangesApplied(); + } + callCount++; + }); + + try { + let runner = new Zotero.Sync.Runner_Module({ baseURL, apiKey }); + await runner._sync({ + libraries: [library.libraryID], + onError: e => { throw e; }, + restartSync: true + }); + } + finally { + engineStub.restore(); + } + + assert.equal(callCount, 2, + "sanity: engine.start ran twice (initial run + restart)"); + assert.isFalse(Zotero.UndoHistory.canUndo(), + "undo stack should be cleared even when restartSync recurses"); + }); + }); +});