diff --git a/chrome/content/zotero/collectionTree.jsx b/chrome/content/zotero/collectionTree.jsx index a068430af4..0583244036 100644 --- a/chrome/content/zotero/collectionTree.jsx +++ b/chrome/content/zotero/collectionTree.jsx @@ -283,7 +283,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); } @@ -1330,14 +1330,27 @@ var CollectionTree = class CollectionTree extends LibraryTree { await row.ref.eraseTx(); } if (others.length) { + let collectionCount = others.filter(r => r.isCollection()).length; + let searchCount = others.length - collectionCount; + // Use the search label only when nothing but searches is selected; + // otherwise describe the action in terms of collections + let undoAction, undoActionArgs; + if (searchCount && !collectionCount) { + undoAction = 'undo-action-trash-search'; + undoActionArgs = { count: searchCount }; + } + else { + undoAction = 'undo-action-trash-collection'; + undoActionArgs = { count: collectionCount + searchCount }; + } await Zotero.DB.executeTransaction(async () => { for (let row of others) { row.ref.deleted = true; if (row.isCollection()) { - await row.ref.save({ deleteItems }); + await row.ref.save({ deleteItems, undoAction, undoActionArgs }); } else { - await row.ref.save(); + await row.ref.save({ undoAction, undoActionArgs }); } } }); @@ -2260,16 +2273,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(); @@ -2304,7 +2322,7 @@ var CollectionTree = class CollectionTree extends LibraryTree { await Zotero.DB.executeTransaction(async () => { for (let droppedCollection of droppedCollections) { droppedCollection.parentID = targetCollectionID; - await droppedCollection.save(); + await droppedCollection.save({ undoAction: 'undo-action-move-collection' }); } }); } @@ -2380,6 +2398,21 @@ 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 } + ); + } + else { + Zotero.UndoHistory.stageAction( + 'undo-action-add-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 069b9bf1f6..5248d02d58 100644 --- a/chrome/content/zotero/collectionViewItemTree.jsx +++ b/chrome/content/zotero/collectionViewItemTree.jsx @@ -1562,6 +1562,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; @@ -1576,6 +1581,11 @@ class CollectionViewItemTree extends ItemTree { // Add to all selected collections if (collectionRows.length) { await Zotero.DB.executeTransaction(async function () { + Zotero.UndoHistory.stageAction( + 'undo-action-add-to-collection', + { count: items.length } + ); + for (let i = 0; i < items.length; i++) { let item = items[i]; var source = item.isRegularItem() ? false : item.parentItemID; @@ -1594,12 +1604,15 @@ class CollectionViewItemTree extends ItemTree { // Only library roots selected -- remove from parent and make top-level else 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(); } }); } @@ -1894,15 +1907,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 @@ -2038,6 +2057,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 a1a2cfc382..4c1e519242 100644 --- a/chrome/content/zotero/containers/tagSelectorContainer.jsx +++ b/chrome/content/zotero/containers/tagSelectorContainer.jsx @@ -739,7 +739,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 06d9cb493b..f6e05820cc 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(); @@ -2096,7 +2102,7 @@ return; } this.item.removeCreator(index); - await this.item.saveTx(); + await this.item.saveTx({ undoAction: 'undo-action-remove-creator' }); } removeUnsavedCreatorRow(onlyIfEmpty = false) { @@ -2301,11 +2307,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; }); } @@ -2396,7 +2402,7 @@ this._selectField = `itembox-field-value-creator-${newCreator.position}-lastName`; if (this.saveOnEdit) { - this.item.saveTx(); + this.item.saveTx({ undoAction: 'undo-action-edit-creator' }); } } } @@ -2469,10 +2475,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); @@ -2546,7 +2556,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); } } @@ -2582,7 +2605,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 @@ -2590,9 +2613,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) { @@ -2621,10 +2644,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() { @@ -2724,7 +2753,7 @@ this.modifyCreator(creatorIndex, fields); if (this.saveOnEdit) { - await this.item.saveTx(); + await this.item.saveTx({ undoAction: 'undo-action-edit-creator' }); } } @@ -2747,7 +2776,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' }); } } @@ -2861,7 +2890,7 @@ this.item.setCreator(i, creators[i]); } if (this.saveOnEdit && !skipSave) { - this.item.saveTx(); + this.item.saveTx({ undoAction: 'undo-action-reorder-creator' }); } } @@ -3217,7 +3246,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 1c287fc1ac..1a322ff4bc 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..af4057b59e 100644 --- a/chrome/content/zotero/xpcom/data/dataObject.js +++ b/chrome/content/zotero/xpcom/data/dataObject.js @@ -884,6 +884,96 @@ 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 + }; +}; + + +/** + * Whether the object still holds the values captured in an undo snapshot for + * the given side -- the read-and-compare counterpart of _getUndoData(). Used by + * Zotero.UndoHistory to avoid replaying a snapshot over an external change. + * + * @param {Object} fields -- a change record's `fields` map (field -> { old, new }) + * @param {String} side -- 'new' (undo) or 'old' (redo) + * @return {Boolean} -- true if every field still matches the recorded value + */ +Zotero.DataObject.prototype.matchesUndoSnapshot = function (fields, side) { + for (let field of Object.keys(fields)) { + if (!this._undoFieldMatches(field, fields[field][side])) { + return false; + } + } + return true; +}; + + +/** + * Whether a single field still holds a recorded undo value. Overridden by + * Zotero.Item for item-specific fields; the base handles the primary scalar + * fields shared by all data objects. + * + * @param {String} field + * @param {*} recorded -- the recorded value for the side being checked + * @return {Boolean} + */ +Zotero.DataObject.prototype._undoFieldMatches = function (field, recorded) { + if (field === 'deleted') { + return this._deleted === recorded; + } + if (field === 'name') { + return this._name === recorded; + } + if (field === 'parentKey') { + return this._parentKey === recorded; + } + return this['_' + field] === recorded; +}; + + /** * @param {String} [op='edit'] - Operation to check; if not provided, check edit privileges for * library @@ -907,6 +997,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 */ @@ -934,17 +1029,17 @@ Zotero.DataObject.prototype.save = async function (options = {}) { ].forEach(x => env.options[x] = true); } - var proceed = await this._initSave(env); - if (!proceed) return false; - - if (env.isNew) { - Zotero.debug('Saving data for new ' + this._objectType + ' to database', 4); - } - else { - Zotero.debug('Updating database with new ' + this._objectType + ' data', 4); - } - try { + var proceed = await this._initSave(env); + if (!proceed) return false; + + if (env.isNew) { + Zotero.debug('Saving data for new ' + this._objectType + ' to database', 4); + } + else { + Zotero.debug('Updating database with new ' + this._objectType + ' data', 4); + } + if (Zotero.DataObject.prototype._finalizeSave == this._finalizeSave) { throw new Error("_finalizeSave not implemented for Zotero." + this._ObjectType); } @@ -969,7 +1064,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 +1079,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 +1095,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..3362b98596 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -880,6 +880,217 @@ 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 + }; +}; + + +/** + * @see Zotero.DataObject.prototype._undoFieldMatches + * + * Mirrors how _getUndoData() (above) captures each item field, and reuses the + * canonical change-detection helpers so the staleness check and save-time + * change detection stay in agreement. + */ +Zotero.Item.prototype._undoFieldMatches = function (field, recorded) { + switch (field) { + case 'collections': + return !Zotero.DataObjectUtilities._collectionsChanged(this._collections, recorded); + + case 'tags': + if (!Array.isArray(recorded)) { + return this._tags === recorded; + } + return !Zotero.DataObjectUtilities._tagsChanged(this._tags, recorded); + + case 'relations': + return this._undoRelationsMatch(recorded); + + case 'creators': + return this._undoCreatorsMatch(recorded); + + case 'note': + return this._noteText === recorded; + + case 'itemTypeID': + return this._itemTypeID === recorded; + } + + let fieldID = Zotero.ItemFields.getID(field); + if (fieldID) { + return this._undoItemDataMatches(fieldID, recorded); + } + // Primary scalar field (e.g. dateAdded) -- defer to the base implementation + return Zotero.DataObject.prototype._undoFieldMatches.call(this, field, recorded); +}; + + +/** + * Compare a recorded item-data value against the live one. An empty field reads + * back as false, null, undefined, or '' depending on the path, so treat all of + * those as equal -- like setField()'s own change check -- to avoid mistaking an + * unchanged value for an external edit. + * + * @param {Integer} fieldID + * @param {*} recorded + * @return {Boolean} + */ +Zotero.Item.prototype._undoItemDataMatches = function (fieldID, recorded) { + let current = this._itemData ? this._itemData[fieldID] : undefined; + let emptyCurrent = current === undefined || current === null || current === false || current === ''; + let emptyRecorded = recorded === undefined || recorded === null || recorded === false || recorded === ''; + if (emptyCurrent || emptyRecorded) { + return emptyCurrent === emptyRecorded; + } + return current === recorded; +}; + + +/** + * Index-keyed creator comparison (reordering counts as a change), against the + * { index -> creatorData } shape _getUndoData() records. + * + * @param {Object} recorded + * @return {Boolean} + */ +Zotero.Item.prototype._undoCreatorsMatch = function (recorded) { + recorded = recorded || {}; + if (this._creators.length !== Object.keys(recorded).length) { + return false; + } + for (let i = 0; i < this._creators.length; i++) { + if (!Zotero.Creators.equals(this._creators[i], recorded[i])) { + return false; + } + } + return true; +}; + + +/** + * Order-independent comparison of the flat [predicate, object] pair arrays + * _getUndoData() records for relations. + * + * @param {Array} recorded + * @return {Boolean} + */ +Zotero.Item.prototype._undoRelationsMatch = function (recorded) { + if (!Array.isArray(recorded)) { + return false; + } + let current = this._relations.map(r => [...r]); + if (current.length !== recorded.length) { + return false; + } + let key = pair => pair[0] + "\t" + pair[1]; + return Zotero.Utilities.arrayEquals(current.map(key).sort(), recorded.map(key).sort()); +}; + + /* * Get the title for an item for display in the interface * @@ -1540,6 +1751,14 @@ Zotero.Item.prototype._saveData = async function (env) { if (Zotero.ItemFields.getID('accessDate') == fieldID && (this.getField(fieldID)) == 'CURRENT_TIMESTAMP') { value = Zotero.DB.transactionDateTime; + // The undo snapshot captured the unresolved sentinel as this + // field's 'new' value. Replace it with the timestamp we're + // actually writing so staleness detection can compare against + // the stored value once the item reloads it + if (env.undoData && env.undoData.fields.accessDate + && env.undoData.fields.accessDate.new === 'CURRENT_TIMESTAMP') { + env.undoData.fields.accessDate.new = value; + } } let valueID = await Zotero.DB.valueQueryAsync(valueSQL, [value], { debug: true }) diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index 99a6fd4f65..1c78dd67e4 100644 --- a/chrome/content/zotero/xpcom/data/items.js +++ b/chrome/content/zotero/xpcom/data/items.js @@ -1012,10 +1012,11 @@ Zotero.Items = function () { this.trash = async function (ids) { 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) { @@ -1023,18 +1024,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 => { diff --git a/chrome/content/zotero/xpcom/data/library.js b/chrome/content/zotero/xpcom/data/library.js index 225a8c3264..3d43d0704e 100644 --- a/chrome/content/zotero/xpcom/data/library.js +++ b/chrome/content/zotero/xpcom/data/library.js @@ -683,6 +683,13 @@ Zotero.Library.prototype._eraseData = async function (env) { await Zotero.DB.queryAsync("DELETE FROM libraries WHERE libraryID=?", this.libraryID); // TODO: Emit event so this doesn't have to be here await Zotero.Fulltext.clearLibraryVersion(this.libraryID); + + // Discard undo/redo history that references this library + if (Zotero.UndoHistory) { + Zotero.DB.addCurrentCallback('commit', function () { + Zotero.UndoHistory.clearForLibrary(this.libraryID); + }.bind(this)); + } }; Zotero.Library.prototype._finalizeErase = async function (env) { 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 0473486643..0c8d78001a 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 dd07bd97b9..88900f92df 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -34,7 +34,21 @@ Zotero.Sync.Data.Local = { _loginManagerRealmLegacy: '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) { @@ -354,7 +368,7 @@ Zotero.Sync.Data.Local = { var library = Zotero.Libraries.get(libraryID); library.libraryVersion = -1; await library.saveTx(); - + await this.resetUnsyncedLibraryFiles(libraryID); }, @@ -414,8 +428,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); @@ -1512,6 +1527,7 @@ Zotero.Sync.Data.Local = { await obj.erase({ notifierQueue }); + Zotero.Sync.Data.Local.markRemoteChangesApplied(); } catch (e) { results.push({ @@ -1685,6 +1701,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 2c4e5dbbdc..4473fb0bae 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..dcc6137b61 --- /dev/null +++ b/chrome/content/zotero/xpcom/undoHistory.js @@ -0,0 +1,519 @@ +/* + ***** 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, + _opQueue: Promise.resolve(), + + init() { + // default (100) when unset or non-numeric. 0 (or less) disables undo/redo entirely. + let steps = Zotero.Prefs.get('undoHistory.steps'); + this._maxSteps = Number.isInteger(steps) ? steps : 100; + this.clear(); + }, + + /** + * @return {Boolean} + */ + isEnabled() { + return this._maxSteps > 0; + }, + + clear() { + this._undoStack = []; + this._redoStack = []; + this._pendingEntry = null; + }, + + /** + * Discard both stacks if any entry references an object in the given library. + * @param {Integer} libraryID + */ + clearForLibrary(libraryID) { + let affectsLibrary = entry => entry.changes.some(change => change.libraryID === libraryID); + if (this._undoStack.some(affectsLibrary) || this._redoStack.some(affectsLibrary)) { + this.clear(); + } + }, + + /** + * 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) { + return { + supportsCommand: cmd => 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 (this._hasNativeCommand(doc, cmd)) return false; + if (cmd === 'cmd_undo') return this.canUndo(); + if (cmd === 'cmd_redo') return this.canRedo(); + return false; + }, + doCommand: (cmd) => { + if (cmd === 'cmd_undo') this.undo(); + else if (cmd === 'cmd_redo') this.redo(); + }, + onEvent: () => {} + }; + }, + + canUndo() { + return this._undoStack.length > 0; + }, + + canRedo() { + return this._redoStack.length > 0; + }, + + /** + * Run an undo/redo operation only after all previously queued ones have + * settled, so each step's staleness check sees the fully committed result + * of the step before it. Returns the operation's own result; a failure in + * one operation doesn't stall the queue for the next. + * + * @param {Function} fn -- async operation returning Promise + * @return {Promise} + */ + _enqueue(fn) { + this._opQueue = this._opQueue.then(fn, fn); + return this._opQueue; + }, + + /** + * 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. Serialized through a queue. + * + * @return {Promise} -- true if an entry was undone + */ + undo() { + return this._enqueue(() => this._undo()); + }, + + _undo() { + // Undo restores the 'old' snapshot; decline if a covered object no + // longer holds the recorded 'new' value (an outside writer changed it). + return this._apply({ + fromStack: '_undoStack', + toStack: '_redoStack', + staleSide: 'new', + applySide: 'old', + label: 'undo' + }); + }, + + /** + * Redo the most recently undone entry. Serialized through the same queue as undo() + * + * @return {Promise} -- true if an entry was redone + */ + redo() { + return this._enqueue(() => this._redo()); + }, + + _redo() { + // Redo reapplies the 'new' snapshot; decline if a covered object no + // longer holds the recorded 'old' value (mirrors the check in _undo). + return this._apply({ + fromStack: '_redoStack', + toStack: '_undoStack', + staleSide: 'old', + applySide: 'new', + label: 'redo' + }); + }, + + /** + * Pop the top entry off one stack, write back the recorded snapshot for the + * given side, and -- on success -- push the entry onto the opposite stack. + * Shared implementation behind _undo() (applySide 'old') and _redo() + * (applySide 'new'). + * + * The staleness check and the apply share one transaction so they're atomic. + * If any object no longer holds its recorded `staleSide` value, an outside + * writer changed it and replaying would clobber that change, so we decline + * and discard history. A mid-apply failure is likewise untrustworthy, so we + * realign memory with the rolled-back DB and discard. + * + * @param {Object} opts + * @param {String} opts.fromStack -- name of the stack to pop the entry from + * @param {String} opts.toStack -- name of the stack to push the entry to on success + * @param {String} opts.staleSide -- recorded side the object must still hold ('new'/'old') + * @param {String} opts.applySide -- recorded side to write back ('old'/'new') + * @param {String} opts.label -- 'undo' or 'redo', for debug logging + * @return {Promise} -- true if an entry was applied + */ + async _apply({ fromStack, toStack, staleSide, applySide, label }) { + let entry = this[fromStack].pop(); + if (!entry) return false; + let stale = false; + try { + await Zotero.DB.executeTransaction(async () => { + if (this._entryIsStale(entry, staleSide)) { + stale = true; + return; + } + for (let change of entry.changes) { + let obj = this._getObject(change); + if (!obj) continue; + // Apply itemTypeID first so setType() migrates fields before + // the type-specific fields are restored on the correct type + if (change.fields.itemTypeID) { + this._applyFieldValue(obj, 'itemTypeID', change.fields.itemTypeID[applySide]); + } + for (let [field, values] of Object.entries(change.fields)) { + if (field === 'itemTypeID') continue; + this._applyFieldValue(obj, field, values[applySide]); + } + await obj.save({ skipSelect: true }); + } + }); + if (stale) { + Zotero.debug(`UndoHistory: declining stale ${label} entry`); + this.clear(); + return false; + } + this[toStack].push(entry); + return true; + } + catch (e) { + Zotero.debug(`UndoHistory: ${label} failed: ` + e); + // Realign memory with the rolled-back DB before clearing history. + await this._reloadEntryObjects(entry); + // A failure means the object drifted out from under our snapshots, + // so the rest of the stack can't be trusted either. Discard history + // rather than risk applying stale values. + this.clear(); + return false; + } + }, + + // -- 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) { + if (!this.isEnabled()) { + return; + } + 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) { + if (!this.isEnabled()) { + return; + } + 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); + } + }, + + /** + * Realign in-memory state with the DB after a failed apply. The apply runs + * many saves in one transaction; if a later one throws, the transaction + * rolls back, but objects saved earlier already hold their reverted values + * in memory. Reload each covered object so memory matches the committed DB. + * Objects that no longer resolve (e.g. erased) or fail to reload are skipped. + * + * @param {Object} entry + */ + async _reloadEntryObjects(entry) { + for (let change of entry.changes) { + let obj = this._getObject(change); + if (!obj) { + continue; + } + try { + await obj.reload(null, true); + } + catch (e) { + Zotero.debug('UndoHistory: failed to reload object after apply failure: ' + e); + } + } + }, + + /** + * 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; + } + }, + + /** + * Whether an object covered by the entry no longer holds the value we + * recorded for the given side ('new' for undo, 'old' for redo) -- meaning + * something outside this history changed it and replaying would clobber that + * change. Objects that no longer resolve (e.g. erased) are skipped. + * + * @param {Object} entry + * @param {String} side -- 'new' (undo) or 'old' (redo) + * @return {Boolean} + */ + _entryIsStale(entry, side) { + for (let change of entry.changes) { + let obj = this._getObject(change); + if (!obj) { + continue; + } + + let matches; + try { + matches = obj.matchesUndoSnapshot(change.fields, side); + } + catch (e) { + // Can't verify the snapshot, so we can't rule out an external change + Zotero.debug('UndoHistory: could not verify snapshot for staleness; declining entry: ' + e); + return true; + } + if (!matches) { + return true; + } + } + return false; + } +}; 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 3ec8d7a7a0..2af2386810 100644 --- a/chrome/content/zotero/zotero.mjs +++ b/chrome/content/zotero/zotero.mjs @@ -112,6 +112,7 @@ const xpcomFilesLocal = [ 'locateManager', 'mime', 'notifier', + 'undoHistory', 'fileHandlers', 'osKeyStore', 'plugins', diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 450584500d..ac56728aa2 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -105,7 +105,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', () => { @@ -2579,6 +2583,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]; @@ -2716,6 +2721,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) { @@ -2801,6 +2818,7 @@ var ZoteroPane = new function () { Zotero.hideZoteroPaneOverlays(); } await Zotero.purgeDataObjects(); + Zotero.UndoHistory.clear(); } }; @@ -2856,7 +2874,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. @@ -4706,8 +4724,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); + }); }; @@ -5612,6 +5636,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) { @@ -5634,6 +5663,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 }); @@ -6695,12 +6729,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 dd12120aca..356aab8ff1 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/chrome/locale/en-US/zotero/zotero.ftl @@ -1073,3 +1073,88 @@ item-pane-batch-editing-header = { $count -> item-pane-batch-editing-done = .label = { general-done } + +undo-action-edit-metadata = { $count -> + [one] Edit Metadata + *[other] Edit Metadata for { $count } Items +} +undo-action-edit-field = { $count -> + [one] Edit of “{ $field }” + *[other] Edit of “{ $field }” for { $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 e9caa7242b..6ecac853dd 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/collectionTreeTest.js b/test/tests/collectionTreeTest.js index 27d1f2d554..92e7a27d95 100644 --- a/test/tests/collectionTreeTest.js +++ b/test/tests/collectionTreeTest.js @@ -995,13 +995,13 @@ describe("Zotero.CollectionTree", function () { assert.equal(zp.itemsView.rowCount, 0); await select(win, collection2); - + // Target collection should have item assert.equal(zp.itemsView.rowCount, 1); var treeRow = zp.itemsView.getRow(0); assert.equal(treeRow.ref.id, item.id); }); - + it("should add a multiple-library item selection to a collection, copying out-of-library items", async function () { await Zotero.Users.setCurrentUserID(1); await Zotero.Users.setName(1, 'Name'); @@ -1080,6 +1080,41 @@ describe("Zotero.CollectionTree", function () { assert.isFalse(await canDrop('item', 'L' + userLibraryID, [item1.id, item2.id])); }); + it("should record an undo step when adding an item to a collection", async function () { + var collection = await createDataObject('collection'); + var item = await createDataObject('item', false, { skipSelect: true }); + Zotero.UndoHistory.clear(); + + // Add observer to wait for collection add + var deferred = Zotero.Promise.defer(); + var observerID = Zotero.Notifier.registerObserver({ + notify: function (event, type, ids, extraData) { + if (type == 'collection-item' && event == 'add' + && ids[0] == collection.id + "-" + item.id) { + setTimeout(function () { + deferred.resolve(); + }); + } + } + }, 'collection-item', 'test'); + + await onDrop('item', 'C' + collection.id, [item.id], deferred.promise); + + Zotero.Notifier.unregisterObserver(observerID); + + assert.include(item.getCollections(), collection.id); + assert.isTrue(Zotero.UndoHistory.canUndo()); + var action = Zotero.UndoHistory.getUndoAction(); + assert.equal(action.action, 'undo-action-add-to-collection'); + assert.equal(action.actionArgs.count, 1); + + await Zotero.UndoHistory.undo(); + assert.notInclude(item.getCollections(), collection.id); + + await Zotero.UndoHistory.redo(); + assert.include(item.getCollections(), collection.id); + }); + describe("My Publications", function () { function getItemModifyPromise(item) { // Add observer to wait for item modification diff --git a/test/tests/itemPaneTest.js b/test/tests/itemPaneTest.js index 6a45461756..32b44b7ac0 100644 --- a/test/tests/itemPaneTest.js +++ b/test/tests/itemPaneTest.js @@ -3081,6 +3081,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 = doc.getElementById('batch-edit-prompt-enable'); + 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, Zotero.getString('item-pane-batch-editing-multiple-values-placeholder'), "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, Zotero.getString('item-pane-batch-editing-multiple-values-placeholder'), "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"; @@ -3144,6 +3220,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 = doc.getElementById('batch-edit-prompt-enable'); + 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..56973a79ad --- /dev/null +++ b/test/tests/undoHistoryTest.js @@ -0,0 +1,1579 @@ +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("apply failure", function () { + it("should clear both stacks if applying an undo entry fails", async function () { + let collection = await createDataObject('collection', { name: 'Original' }); + + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + // Force the save during undo to fail + let stub = sinon.stub(collection, 'save').rejects(new Error('save failed')); + let result; + try { + result = await Zotero.UndoHistory.undo(); + } + finally { + stub.restore(); + } + + // Nothing was applied, so undo() should report failure + assert.isFalse(result); + assert.isFalse(Zotero.UndoHistory.canUndo()); + assert.isFalse(Zotero.UndoHistory.canRedo()); + }); + + it("should clear both stacks if applying a redo entry fails", async function () { + let collection = await createDataObject('collection', { name: 'Original' }); + + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + await Zotero.UndoHistory.undo(); + assert.isTrue(Zotero.UndoHistory.canRedo()); + + // Force the save during redo to fail + let stub = sinon.stub(collection, 'save').rejects(new Error('save failed')); + let result; + try { + result = await Zotero.UndoHistory.redo(); + } + finally { + stub.restore(); + } + + // Nothing was applied, so redo() should report failure + assert.isFalse(result); + assert.isFalse(Zotero.UndoHistory.canUndo()); + assert.isFalse(Zotero.UndoHistory.canRedo()); + }); + + it("should not leave an object unsaveable after an undo apply failure", async function () { + // A (move target), P (original parent), B (child being moved) + let collectionA = await createDataObject('collection', { name: 'A' }); + let collectionP = await createDataObject('collection', { name: 'P' }); + let collectionB = await createDataObject('collection', { name: 'B', parentID: collectionP.id }); + + // Move B from P onto A, recording an undo entry for the parent change + Zotero.UndoHistory.clear(); + collectionB.parentID = collectionA.id; + await collectionB.saveTx({ undoAction: 'undo-action-move-collection' }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + // Permanently erase P so its key no longer resolves to a collection + await collectionP.eraseTx(); + + // Undoing tries to set B's parent back to the now-erased P, which makes + // Collection._initSave throw. The apply fails and history is cleared, but + // B must not be left pinned to the vanished parent. + await Zotero.UndoHistory.undo(); + + // B should have been rolled back to its last valid parent (A) and remain + // editable + assert.equal(collectionB.parentID, collectionA.id); + collectionB.name = 'B renamed'; + await collectionB.saveTx(); + assert.equal(collectionB.name, 'B renamed'); + }); + + it("should not leave an earlier object's memory diverged from the DB when a later save fails", async function () { + // Two items edited together as a single batch (one undo entry) + 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.isTrue(Zotero.UndoHistory.canUndo()); + + // item1 is applied and saved first; force the *second* object's save to + // fail so the transaction rolls back only after item1 has already been + // written and reloaded into memory with its undone value. + let stub = sinon.stub(item2, 'save').rejects(new Error('save failed')); + try { + await Zotero.UndoHistory.undo(); + } + finally { + stub.restore(); + } + + // The transaction rolled back, so the DB still holds the committed batch + // value. item1's in-memory state must match the DB rather than retaining + // the rolled-back undo value. + let dbTitle = await Zotero.DB.valueQueryAsync( + "SELECT value FROM itemData JOIN itemDataValues USING (valueID) " + + "WHERE itemID=? AND fieldID=?", + [item1.id, Zotero.ItemFields.getID('title')] + ); + assert.equal(dbTitle, 'Batch Title', "sanity: rollback kept the batch value in the DB"); + assert.equal(item1.getField('title'), 'Batch Title', + "earlier object's memory should match the rolled-back DB, not the undone value"); + assert.isFalse(item1.hasChanged(), + "earlier object should not be left with phantom uncommitted changes"); + }); + }); + + describe("library erasure", function () { + it("should clear undo history when a related library is erased", async function () { + let group = await createGroup(); + let collection = await createDataObject( + 'collection', { libraryID: group.libraryID, name: 'Group Collection' } + ); + Zotero.UndoHistory.clear(); + + collection.name = 'Renamed'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + // The group's objects are cascade-deleted without per-object events, so + // the related undo entry must be discarded + await group.eraseTx(); + assert.isFalse(Zotero.UndoHistory.canUndo()); + assert.isFalse(Zotero.UndoHistory.canRedo()); + }); + + it("should clear undo history when the erased library is referenced only in the redo stack", async function () { + let group = await createGroup(); + let collection = await createDataObject( + 'collection', { libraryID: group.libraryID, name: 'Group Collection' } + ); + Zotero.UndoHistory.clear(); + + collection.name = 'Renamed'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + // Move the entry onto the redo stack + await Zotero.UndoHistory.undo(); + assert.isTrue(Zotero.UndoHistory.canRedo()); + + await group.eraseTx(); + assert.isFalse(Zotero.UndoHistory.canRedo()); + assert.isFalse(Zotero.UndoHistory.canUndo()); + }); + + it("should preserve undo history when an unrelated library is erased", async function () { + // Record an undo entry in the user library + let collection = await createDataObject('collection', { name: 'My Library Collection' }); + Zotero.UndoHistory.clear(); + collection.name = 'Renamed'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + // Erasing an unrelated group must not touch the user-library history + let group = await createGroup(); + await group.eraseTx(); + assert.isTrue(Zotero.UndoHistory.canUndo()); + }); + }); + + 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 + ); + }); + }); + + it("should not drop undo history when undo is requested during an undo", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + item.setField('title', 'Second'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + item.setField('title', 'Third'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + assert.equal(item.getField('title'), 'Third'); + + let unblockFirstUndo = Zotero.Promise.defer(); + let firstUndoReachedTransaction = Zotero.Promise.defer(); + let executeTransaction = Zotero.DB.executeTransaction; + let firstCall = true; + let stub = sinon.stub(Zotero.DB, 'executeTransaction').callsFake(async function (func, options) { + if (firstCall) { + firstCall = false; + firstUndoReachedTransaction.resolve(); + await unblockFirstUndo.promise; + } + return executeTransaction.call(this, func, options); + }); + + try { + let firstUndo = Zotero.UndoHistory.undo(); + await firstUndoReachedTransaction.promise; + let secondUndo = Zotero.UndoHistory.undo(); + unblockFirstUndo.resolve(); + await firstUndo; + await secondUndo; + } + finally { + stub.restore(); + } + + // Both undos should apply in turn (Third -> Second -> Original). Today the + // second undo's synchronous staleness check runs while the first undo is + // still parked in its transaction, so the still-current 'Third' makes the + // second entry look stale and the whole history is cleared -- leaving the + // title at 'Second'. Undo/redo need to be serialized so each step applies. + assert.equal(item.getField('title'), 'Original', + "a second undo issued mid-transaction should apply in turn, not be discarded as stale"); + }); + + describe("stale snapshot application", function () { + it("should not apply an undo snapshot over a concurrent committed edit", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + item.setField('title', 'User Edit'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + let activeTransactionStarted = Zotero.Promise.defer(); + let allowActiveTransactionToChangeItem = Zotero.Promise.defer(); + let activeTransaction = Zotero.DB.executeTransaction(async () => { + activeTransactionStarted.resolve(); + await allowActiveTransactionToChangeItem.promise; + item.setField('title', 'Concurrent Edit'); + await item.save(); + }); + + await activeTransactionStarted.promise; + let undo = Zotero.UndoHistory.undo(); + await Zotero.Promise.delay(1); + allowActiveTransactionToChangeItem.resolve(); + await activeTransaction; + await undo; + + assert.equal(item.getField('title'), 'Concurrent Edit', + "undo should not clobber a change committed while it was waiting for the DB transaction"); + }); + + it("should not clobber a later third-party change to the same field", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + // User edit, recorded on the undo stack as Original -> User Edit + item.setField('title', 'User Edit'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + // A plugin (or any non-UI writer) then changes the same field. No + // undoAction, so no new entry is created -- but the existing entry's + // snapshot (new: 'User Edit') no longer matches the object + item.setField('title', 'Plugin Edit'); + await item.saveTx(); + + await Zotero.UndoHistory.undo(); + + // Undo should detect that the field drifted from the recorded value + // and decline (or clear), not silently revert the plugin's change + assert.equal(item.getField('title'), 'Plugin Edit', + "undo should not apply a stale snapshot over a later change"); + }); + + it("should not clobber a later third-party change when redoing", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + // User edit, recorded on the undo stack as Original -> User Edit + item.setField('title', 'User Edit'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + + // Undo it, moving the entry onto the redo stack. The redo entry now + // expects the field to still read its recorded 'old' value (Original) + await Zotero.UndoHistory.undo(); + assert.equal(item.getField('title'), 'Original'); + assert.isTrue(Zotero.UndoHistory.canRedo()); + + // A plugin then changes the same field. No undoAction, so no new entry + // is created, and the redo stack is preserved -- but the object no + // longer holds the value the redo entry expects + item.setField('title', 'Plugin Edit'); + await item.saveTx(); + + await Zotero.UndoHistory.redo(); + + // Redo should detect that the field drifted from the recorded value and + // decline, not silently replay 'User Edit' over the plugin's change + assert.equal(item.getField('title'), 'Plugin Edit', + "redo should not apply a stale snapshot over a later change"); + }); + + it("should not clobber a later third-party tag change", async function () { + let item = await createDataObject('item', { title: 'Tagged' }); + Zotero.UndoHistory.clear(); + + // User edit, recorded on the undo stack as [] -> ['user-tag'] + item.addTag('user-tag'); + await item.saveTx({ undoAction: 'undo-action-add-tag', undoActionArgs: { count: 1 } }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + // A plugin then swaps that tag for a different one with no undoAction: + // the tag count is unchanged, so detection relies on comparing tag + // identities rather than just the set size + item.removeTag('user-tag'); + item.addTag('plugin-tag'); + await item.saveTx(); + + await Zotero.UndoHistory.undo(); + + // Undoing would reapply the recorded pre-edit tags (none), wiping the + // plugin's tag. Detection should see the tags drifted and decline. + assert.isTrue(item.hasTag('plugin-tag'), + "undo should not clobber the later third-party tag"); + assert.isFalse(Zotero.UndoHistory.canUndo(), + "a detected stale entry should be cleared"); + }); + + it("should still apply undo when a third party only reordered the tags", async function () { + let item = await createDataObject('item', { title: 'Tagged' }); + Zotero.UndoHistory.clear(); + + // User edit recorded on the undo stack: [] -> ['alpha', 'beta', 'gamma'] + // (the recorded snapshot is sorted by setTags) + item.addTag('alpha'); + item.addTag('beta'); + item.addTag('gamma'); + await item.saveTx({ undoAction: 'undo-action-add-tag', undoActionArgs: { count: 3 } }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + // Simulate a third party that persisted the same three tags in a + // different order. `_tags` is loaded in DB order while the snapshot is + // sorted, so a reordered current state is a real possibility; the + // staleness check compares the in-memory tags, so the simulated order + // is what gets compared. + let cached = Zotero.Items.get(item.id); + cached._tags = [{ tag: 'gamma' }, { tag: 'alpha' }, { tag: 'beta' }]; + await Zotero.UndoHistory.undo(); + + // Same set of tags, only reordered -> not a drift -> undo proceeds and + // reverts to the recorded pre-edit state (no tags) + assert.isFalse(item.hasTag('alpha')); + assert.isFalse(item.hasTag('beta')); + assert.isFalse(item.hasTag('gamma')); + assert.isTrue(Zotero.UndoHistory.canRedo(), + "undo should have applied, leaving the entry on the redo stack"); + }); + + it("should still undo an accessDate edit recorded as the CURRENT_TIMESTAMP sentinel", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + // Setting accessDate to the CURRENT_TIMESTAMP sentinel resolves to a + // real SQL timestamp at save time, which the save records as the + // entry's 'new' value and reloads into memory. Nothing external + // changed the field, so the staleness check sees the recorded + // timestamp still in place and lets the undo proceed. + item.setField('accessDate', 'CURRENT_TIMESTAMP'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + await Zotero.UndoHistory.undo(); + + // Undo should apply and revert accessDate, not decline as stale and + // throw away the history + assert.isTrue(Zotero.UndoHistory.canRedo(), + "undo should apply, not be declined as stale"); + assert.equal(item.getField('accessDate'), '', + "undo should have reverted accessDate to its pre-edit (empty) value"); + }); + + it("should decline a stale undo when a third party changed accessDate after a sentinel edit", async function () { + let item = await createDataObject('item', { title: 'Original' }); + Zotero.UndoHistory.clear(); + + // User edit: accessDate set via the CURRENT_TIMESTAMP sentinel, which + // the save resolves to a real timestamp and records as the entry's + // 'new' value so it can be compared later + item.setField('accessDate', 'CURRENT_TIMESTAMP'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + // A third party then changes accessDate to a different timestamp with + // no undoAction, so no new entry is recorded -- but the existing + // snapshot's resolved 'new' value no longer matches the field + item.setField('accessDate', '2020-01-01 00:00:00'); + await item.saveTx(); + + await Zotero.UndoHistory.undo(); + + // The field drifted from the recorded (resolved) value, so undo should + // decline rather than clobber the third party's accessDate + assert.equal(item.getField('accessDate'), '2020-01-01 00:00:00', + "undo should not clobber a third-party accessDate change made after a sentinel edit"); + }); + + it("should still redo a type change whose gained field recorded a null old value", async function () { + let caseTypeID = Zotero.ItemTypes.getID('case'); + let filmTypeID = Zotero.ItemTypes.getID('film'); + + let item = await createDataObject('item', { itemType: 'case' }); + Zotero.UndoHistory.clear(); + + // Change type Case -> Film and set a field that exists only on film. + // setType() initializes the newly-valid film fields to null, so the + // edit's recorded 'old' for `distributor` is null (not a real prior + // value). + item.setType(filmTypeID); + item.setField('distributor', 'Acme Pictures'); + await item.saveTx({ undoAction: 'undo-action-change-type' }); + assert.equal(item.itemTypeID, filmTypeID); + assert.equal(item.getField('distributor'), 'Acme Pictures'); + + // Undo: Film -> Case, distributor dropped + await Zotero.UndoHistory.undo(); + assert.equal(item.itemTypeID, caseTypeID); + assert.isTrue(Zotero.UndoHistory.canRedo()); + + // Nothing changed the item since the undo, so the entry is not stale + // and the type change should replay. After the undo, distributor is + // invalid on a case item and reads back as `false`, while the entry + // recorded its 'old' as null -- a null-vs-false mismatch that must + // not be mistaken for an external change. + await Zotero.UndoHistory.redo(); + assert.equal(item.itemTypeID, filmTypeID, + "redo should re-apply the type change, not decline as stale"); + assert.equal(item.getField('distributor'), 'Acme Pictures', + "redo should restore the film-only field"); + }); + }); + + 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"); + }); + + it("should not apply a stale snapshot via undo while a sync is still running", async function () { + // The undo stack is only cleared at the end of _sync(), so after a + // remote change has been applied but before the sync finishes, + // undo is still enabled and applies a snapshot that predates the + // remote change, silently reverting it + let library = Zotero.Libraries.userLibrary; + let lastLibraryVersion = 5; + let newLibraryVersion = 6; + library.libraryVersion = library.storageVersion = lastLibraryVersion; + await library.saveTx(); + + // Undoable user edit: Before -> After + let item = await createDataObject('item', { title: 'Before' }); + Zotero.UndoHistory.clear(); + item.setField('title', 'After'); + await item.saveTx({ undoAction: 'undo-action-edit-metadata' }); + item.version = lastLibraryVersion; + await Zotero.Sync.Data.Local.markObjectAsSynced(item); + let itemKey = item.key; + 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 Edit' + }) + }]; + let engineStub = sinon.stub(Zotero.Sync.Data.Engine.prototype, 'start') + .callsFake(async function () { + // Remote change lands on the same item the undo entry covers + await Zotero.Sync.Data.Local.processObjectsFromJSON( + 'item', libraryID, remoteJSON, {} + ); + assert.equal(item.getField('title'), 'Remote Edit', + "sanity: remote data was applied"); + // User presses Cmd+Z while the sync is still in progress + await Zotero.UndoHistory.undo(); + }); + + 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 Edit', + "mid-sync undo should not revert a remote change that was already applied"); + }); + }); + + describe("step limit pref", function () { + afterEach(function () { + Zotero.Prefs.clear('undoHistory.steps'); + Zotero.UndoHistory.init(); + }); + + it("should disable capture when undoHistory.steps is 0", async function () { + Zotero.Prefs.set('undoHistory.steps', 0); + Zotero.UndoHistory.init(); + + let collection = await createDataObject('collection', { name: 'Original' }); + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + + assert.equal(collection.name, 'Modified'); + assert.isFalse(Zotero.UndoHistory.canUndo(), + "a configured step limit of 0 should disable undo/redo, not fall back to the default"); + }); + + it("should cap the undo stack at undoHistory.steps entries", async function () { + Zotero.Prefs.set('undoHistory.steps', 2); + Zotero.UndoHistory.init(); + + let collection = await createDataObject('collection', { name: 'Original' }); + for (let name of ['First', 'Second', 'Third']) { + collection.name = name; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + } + + // Three edits, limit of 2: only the two most recent are undoable + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.isFalse(await Zotero.UndoHistory.undo()); + assert.equal(collection.name, 'First'); + }); + }); +}); diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index b5fe4bc26f..d63a8e9f24 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -862,8 +862,31 @@ describe("ZoteroPane", function () { assert.isTrue(item.deleted); }); }); - - + + + describe("#emptyTrash()", function () { + it("should clear the undo/redo history", async function () { + // Record an undo entry + Zotero.UndoHistory.clear(); + var collection = await createDataObject('collection', { name: 'Original' }); + collection.name = 'Renamed'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + // Put something in the trash to empty + await createDataObject('item', { deleted: true }); + + await selectTrash(win); + var promise = waitForDialog(); + await zp.emptyTrash(); + await promise; + + assert.isFalse(Zotero.UndoHistory.canUndo()); + assert.isFalse(Zotero.UndoHistory.canRedo()); + }); + }); + + describe("#setVirtual()", function () { var cv;