From a64fad3714d904298b506a0eddda087b7cb85194 Mon Sep 17 00:00:00 2001 From: Abe Jellinek <1770299+AbeJellinek@users.noreply.github.com> Date: Fri, 14 Aug 2026 10:34:37 -0400 Subject: [PATCH 1/2] Reader: Mirror undoable actions to global undo history --- .../content/zotero/standalone/standalone.js | 43 +-- chrome/content/zotero/xpcom/reader.js | 91 ++++- chrome/content/zotero/xpcom/undoHistory.js | 315 +++++++++++++++++- chrome/locale/en-US/zotero/zotero.ftl | 20 ++ test/tests/readerTest.js | 225 +++++++++++++ test/tests/undoHistoryTest.js | 231 +++++++++++++ 6 files changed, 864 insertions(+), 61 deletions(-) diff --git a/chrome/content/zotero/standalone/standalone.js b/chrome/content/zotero/standalone/standalone.js index 6b933333fe..f8e3746c40 100644 --- a/chrome/content/zotero/standalone/standalone.js +++ b/chrome/content/zotero/standalone/standalone.js @@ -237,52 +237,11 @@ const ZoteroStandalone = new function () { this.updateQuickCopyOptions(); // goUpdateGlobalEditMenuItems(true) is necessary to update Edit menu when contenteditable is focused window.goUpdateGlobalEditMenuItems(true); - this._updateUndoRedoLabels(); + Zotero.UndoHistory.updateMenuItems(document); 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/reader.js b/chrome/content/zotero/xpcom/reader.js index 87af69b2f5..6cb4d1bb33 100644 --- a/chrome/content/zotero/xpcom/reader.js +++ b/chrome/content/zotero/xpcom/reader.js @@ -46,6 +46,16 @@ const READ_ALOUD_VOICE_DEFAULTS_PATH = PathUtils.join(Zotero.Profile.dir, 'readA // Whether the Read Aloud audio cache has been pruned of stale versions this session let readAloudCachePruned = false; +// Fluent messages for the undoable actions the reader reports +const UNDO_ACTIONS = { + 'add-annotations': 'undo-action-add-annotation', + 'update-annotations': 'undo-action-edit-annotation', + 'delete-annotations': 'undo-action-delete-annotation', + 'convert-annotations': 'undo-action-convert-annotation', + 'merge-annotations': 'undo-action-merge-annotations', +}; +const UNDO_ACTION_FALLBACK = 'undo-action-edit-annotation'; + class ReaderInstance { constructor(options) { this.stateFileName = '.zotero-reader-state'; @@ -404,6 +414,11 @@ class ReaderInstance { onChangeSidebarView: (view) => { Zotero.Prefs.set('reader.lastSidebarTab', view); }, + // Passing this hands undo/redo over to us, so leave it out when the + // undo history is disabled and let the reader keep handling its own + onChangeUndoHistory: Zotero.UndoHistory.isEnabled() + ? history => this._updateUndoHistory(history) + : undefined, onSetPopupPosition: (id, position) => { this._setPopupPosition(id, position); }, @@ -647,6 +662,8 @@ class ReaderInstance { }, }, this._iframeWindow, { cloneFunctions: true })); + this._registerUndoHistoryProvider(); + this._resolveInitPromise(); // Set title once again, because `ReaderWindow` isn't loaded the first time this.updateTitle(); @@ -692,11 +709,62 @@ class ReaderInstance { }; } + /** + * Take part in the app-wide undo history, which owns the Undo/Redo commands + * and interleaves the reader's annotation changes with changes made + * elsewhere. The reader keeps its own annotation snapshots, so it does the + * stepping itself and only reports what its history looks like. + */ + _registerUndoHistoryProvider() { + if (this._isTransient() || !Zotero.UndoHistory.isEnabled()) { + return; + } + Zotero.UndoHistory.registerProvider(this._instanceID, { + libraryID: this._item.libraryID, + window: this._iframeWindow, + undo: () => this._internalReader.undo(), + redo: () => this._internalReader.redo(), + reveal: () => this.reveal() + }); + } + + _updateUndoHistory(history) { + if (this._isTransient()) { + return; + } + try { + let { undoSteps, redoSteps } = JSON.parse(JSON.stringify(history)); + Zotero.UndoHistory.setProviderSteps(this._instanceID, { + undoSteps: undoSteps.map(step => this._toUndoHistoryStep(step)), + redoSteps: redoSteps.map(step => this._toUndoHistoryStep(step)) + }); + } + catch (e) { + // Never let a problem here interfere with annotation editing + Zotero.logError(e); + } + } + + _toUndoHistoryStep({ id, revision, action, count }) { + return { + id, + revision, + action: UNDO_ACTIONS[action] || UNDO_ACTION_FALLBACK, + actionArgs: { count } + }; + } + + /** + * Bring this reader into view. + */ + reveal() {} + uninit() { if (this._isUninitialized) { return; } this._isUninitialized = true; + Zotero.UndoHistory.unregisterProvider(this._instanceID); if (this._customEventHandler && this._iframeWindow) { try { this._iframeWindow.removeEventListener('customEvent', this._customEventHandler); @@ -2075,6 +2143,16 @@ class ReaderTab extends ReaderInstance { } } + reveal() { + if (this._isTabClosed) { + return; + } + this._window.Zotero_Tabs.select(this.tabID); + if (Services.focus.activeWindow !== this._window) { + this._window.focus(); + } + } + _handleLoad = (event) => { if (this._iframe && this._iframe.contentWindow && this._iframe.contentWindow.document === event.target) { this._window.removeEventListener('DOMContentLoaded', this._handleLoad); @@ -2222,6 +2300,12 @@ class ReaderWindow extends ReaderInstance { this._window.onViewMenuOpen = this._onViewMenuOpen.bind(this); this._window.onWindowMenuOpen = this._onWindowMenuOpen.bind(this); this._window.reader = this; + // Register a window controller for undo/redo, as the main window + // does. Appending (rather than inserting at 0) ensures + // text-editing controllers take priority. + this._window.controllers.appendController( + Zotero.UndoHistory.getController(this._window.document) + ); this._iframe = this._window.document.getElementById('reader'); this._iframe.docShell.windowDraggingAllowed = true; } @@ -2253,6 +2337,10 @@ class ReaderWindow extends ReaderInstance { this._onClose(); } + reveal() { + this._window.focus(); + } + _setTitleValue(title) { // Tab titles render Citeproc.js markup. There's no good way // to show rich text in a window title, but we can at least @@ -2292,6 +2380,7 @@ class ReaderWindow extends ReaderInstance { return; } this._window.goUpdateGlobalEditMenuItems(true); + Zotero.UndoHistory.updateMenuItems(this._window.document); this.onUpdateCustomMenus(event, 'edit', popup); } @@ -2882,7 +2971,7 @@ class Reader { getByTabID(tabID) { return this._readers.find(r => (r instanceof ReaderTab) && r.tabID === tabID); } - + getWindowStates() { return this._readers .filter(r => r instanceof ReaderWindow) diff --git a/chrome/content/zotero/xpcom/undoHistory.js b/chrome/content/zotero/xpcom/undoHistory.js index 9feb10cbe1..cc0aee0f1f 100644 --- a/chrome/content/zotero/xpcom/undoHistory.js +++ b/chrome/content/zotero/xpcom/undoHistory.js @@ -40,6 +40,11 @@ * stageAction is opt-in via save({ undoAction, undoActionArgs }) or by an * outer caller invoking Zotero.UndoHistory.stageAction() directly inside * the transaction. + * + * Components that maintain their own history of undoable changes participate + * via registerProvider() and setProviderSteps(). Their steps are interleaved + * with the entries captured here. Providers are responsible for + * undoing/reapplying their changes themselves. */ Zotero.UndoHistory = { _undoStack: [], @@ -47,6 +52,7 @@ Zotero.UndoHistory = { _pendingEntry: null, _maxSteps: 100, _opQueue: Promise.resolve(), + _providers: new Map(), init() { // default (100) when unset or non-numeric. 0 (or less) disables undo/redo entirely. @@ -73,12 +79,158 @@ Zotero.UndoHistory = { * @param {Integer} libraryID */ clearForLibrary(libraryID) { - let affectsLibrary = entry => entry.changes.some(change => change.libraryID === libraryID); + let affectsLibrary = entry => (entry.providerID + ? entry.libraryID === libraryID + : entry.changes.some(change => change.libraryID === libraryID)); if (this._undoStack.some(affectsLibrary) || this._redoStack.some(affectsLibrary)) { this.clear(); } }, + /** + * Register a component that keeps its own history of undoable changes and + * wants those changes to take part in the app-wide undo/redo commands. + * + * The provider reports its history with setProviderSteps() and does the + * actual work in undo() and redo(), each of which steps the provider's + * own history by one and returns whether it did so. + * + * @param {String} providerID + * @param {Object} options + * @param {Integer} options.libraryID Library the provider's changes belong to + * @param {Function} options.undo () => boolean | Promise + * @param {Function} options.redo () => boolean | Promise + * @param {Function} [options.reveal] Bring the provider into view after a step + * @param {Window} [options.window] Frame the provider's changes are made in, + * which therefore shouldn't be left to handle undo/redo itself + */ + registerProvider(providerID, { libraryID, undo, redo, reveal, window }) { + this._providers.set(providerID, + { libraryID, undo, redo, reveal, window, lastStepID: 0 }); + }, + + /** + * Unregister a provider and discard its entries, since nothing can apply + * them anymore + * + * @param {String} providerID + */ + unregisterProvider(providerID) { + if (this._providers.delete(providerID)) { + this._filterEntries(entry => entry.providerID !== providerID); + } + }, + + /** + * Bring a provider's entries in line with the provider's own history. + * + * Steps are ordered oldest first and identified by an id that increases + * monotonically, plus a revision that changes when a step absorbs a later + * change (e.g., continued typing in an annotation comment). Steps the + * provider no longer has are dropped; steps newer than any we've seen are + * pushed onto the undo stack; and a step whose revision changed moves back + * to the top of the stack, since it now covers the most recent change. + * + * @param {String} providerID + * @param {Object} history + * @param {{ id, revision, action, actionArgs }[]} history.undoSteps + * @param {{ id, revision, action, actionArgs }[]} history.redoSteps + */ + setProviderSteps(providerID, { undoSteps = [], redoSteps = [] }) { + let provider = this._providers.get(providerID); + if (!provider || !this.isEnabled()) { + return; + } + let stepIDs = new Set([...undoSteps, ...redoSteps].map(step => step.id)); + this._filterEntries( + entry => entry.providerID !== providerID || stepIDs.has(entry.stepID) + ); + let added = false; + for (let step of undoSteps) { + if (step.id > provider.lastStepID) { + this._undoStack.push({ + providerID, + stepID: step.id, + revision: step.revision, + libraryID: provider.libraryID, + action: step.action, + actionArgs: step.actionArgs || null + }); + added = true; + continue; + } + let index = this._undoStack.findIndex( + entry => entry.providerID === providerID && entry.stepID === step.id); + if (index !== -1 && this._undoStack[index].revision !== step.revision) { + let [entry] = this._undoStack.splice(index, 1); + entry.revision = step.revision; + this._undoStack.push(entry); + } + } + provider.lastStepID = Math.max(provider.lastStepID, ...stepIDs); + if (added) { + this._redoStack = []; + this._trimUndoStack(); + } + }, + + _filterEntries(keep) { + this._undoStack = this._undoStack.filter(keep); + this._redoStack = this._redoStack.filter(keep); + }, + + /** + * Bring the context a change was made in back into view, so the user can see + * what an undo or redo did. Providers know how to reveal themselves; for + * entries we manage, focus the tab where the change was made. + * + * @param {Object} entry + */ + _revealEntry(entry) { + try { + if (entry.providerID) { + let provider = this._providers.get(entry.providerID); + if (provider && provider.reveal) { + provider.reveal(); + } + return; + } + this._revealTab(entry.tabID); + } + catch (e) { + // A change that can't be revealed has still been applied + Zotero.logError(e); + } + }, + + /** + * Select a tab in the main window, bringing the window itself forward if + * it's not the one in front (e.g. when undoing from a reader window) + * + * @param {String} tabID + */ + _revealTab(tabID) { + let win = Zotero.getMainWindow(); + // The tab may have been closed since the change was made + if (!tabID || !win?.Zotero_Tabs?._getTab(tabID).tab) { + return; + } + win.Zotero_Tabs.select(tabID); + if (Services.focus.activeWindow !== win) { + win.focus(); + } + }, + + _getSelectedTabID() { + return Zotero.getMainWindow()?.Zotero_Tabs?.selectedID || null; + }, + + _trimUndoStack() { + if (this._undoStack.length > this._maxSteps) { + this._undoStack.splice(0, this._undoStack.length - this._maxSteps); + } + }, + /** * Return a window controller for cmd_undo/cmd_redo that defers to * native text-editing controllers when they are active. @@ -146,17 +298,22 @@ Zotero.UndoHistory = { }, _hasNativeCommand(doc, cmd) { - // If focus is in a child window (e.g. note-editor or reader iframe), - // it handles its own undo/redo internally + // If focus is in a child frame, it handles its own undo/redo internally + // (e.g. the note editor)... unless it has a provider here, in which case + // its changes are ours to undo and only text editing within it wins let focusedWindow = doc.commandDispatcher.focusedWindow; if (focusedWindow && focusedWindow !== doc.defaultView) { - return true; + if (!this._getProviderForWindow(focusedWindow)) { + return true; + } + return this._isTextBoxFocused(focusedWindow); } 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; + if (['iframe', 'browser'].includes(el.localName) + && !this._getProviderForWindow(el.contentWindow)) { + return true; + } let controllers; try { controllers = el.controllers; @@ -174,6 +331,40 @@ Zotero.UndoHistory = { return false; }, + /** + * The provider that records the changes made in a given frame, if any + * + * @param {Window} win + * @return {Object|undefined} + */ + _getProviderForWindow(win) { + // Collect the frame and its ancestors, since focus may be in a frame + // nested inside the provider's own (e.g., a reader's view iframe) + let frames = []; + for (; win && !frames.includes(win); win = win.parent) { + frames.push(win); + } + for (let provider of this._providers.values()) { + if (provider.window && frames.includes(provider.window)) { + return provider; + } + } + return undefined; + }, + + /** + * Whether the focused element in a frame is a text box, meaning native text + * editing owns undo/redo for it + * + * @param {Window} win + * @return {Boolean} + */ + _isTextBoxFocused(win) { + let el = win.document.activeElement; + return !!el + && (el.isContentEditable || ['input', 'textarea'].includes(el.localName)); + }, + /** * Undo the most recent change entry. Serialized through a queue. * @@ -239,6 +430,9 @@ Zotero.UndoHistory = { async _apply({ fromStack, toStack, staleSide, applySide, label }) { let entry = this[fromStack].pop(); if (!entry) return false; + if (entry.providerID) { + return this._applyProviderEntry(entry, { toStack, label }); + } let stale = false; try { await Zotero.DB.executeTransaction(async () => { @@ -267,6 +461,7 @@ Zotero.UndoHistory = { return false; } this[toStack].push(entry); + this._revealEntry(entry); return true; } catch (e) { @@ -281,6 +476,43 @@ Zotero.UndoHistory = { } }, + /** + * Hand an entry back to the provider that recorded it. The provider holds + * the snapshots and does its own staleness checking, so all that's left + * here is to step it and move the entry to the opposite stack. + * + * A provider that declines (or is gone) is out of step with us, so we drop + * the rest of its entries rather than risk applying them out of order. + * Entries captured here are self-contained, so the stacks are otherwise + * left alone. + * + * @param {Object} entry + * @param {Object} opts + * @param {String} opts.toStack -- name of the stack to push the entry to on success + * @param {String} opts.label -- 'undo' or 'redo' + * @return {Promise} -- true if the entry was applied + */ + async _applyProviderEntry(entry, { toStack, label }) { + let provider = this._providers.get(entry.providerID); + let applied = false; + try { + if (provider) { + applied = !!(await (label === 'undo' ? provider.undo() : provider.redo())); + } + } + catch (e) { + Zotero.debug(`UndoHistory: ${label} failed for provider ${entry.providerID}: ` + e); + } + if (!applied) { + Zotero.debug(`UndoHistory: declining ${label} entry for provider ${entry.providerID}`); + this._filterEntries(other => other.providerID !== entry.providerID); + return false; + } + this[toStack].push(entry); + this._revealEntry(entry); + return true; + }, + // -- Transaction lifecycle callbacks -- _onTransactionBegin(_id) { @@ -293,9 +525,7 @@ Zotero.UndoHistory = { 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._trimUndoStack(); } this._pendingEntry = null; }, @@ -304,6 +534,25 @@ Zotero.UndoHistory = { this._pendingEntry = null; }, + /** + * The entry being staged by the current transaction, created on first use. + * It records the tab the change is being made in, so that undoing it later + * from somewhere else can bring the user back. + * + * @return {Object} + */ + _ensurePendingEntry() { + if (!this._pendingEntry) { + this._pendingEntry = { + changes: [], + action: null, + actionArgs: null, + tabID: this._getSelectedTabID() + }; + } + return this._pendingEntry; + }, + /** * Stage an action label on the pending entry. Must be called inside a * transaction. Together with one or more stageChange() calls in the same @@ -321,11 +570,9 @@ Zotero.UndoHistory = { return; } Zotero.DB.requireTransaction(); - if (!this._pendingEntry) { - this._pendingEntry = { changes: [], action: null, actionArgs: null }; - } - this._pendingEntry.action = action; - this._pendingEntry.actionArgs = actionArgs || null; + let entry = this._ensurePendingEntry(); + entry.action = action; + entry.actionArgs = actionArgs || null; }, /** @@ -350,6 +597,40 @@ Zotero.UndoHistory = { return { action: entry.action, actionArgs: entry.actionArgs }; }, + /** + * Update the Undo/Redo items in a window's Edit menu to name the action + * they would apply (e.g. "Undo Add Tag") + * + * @param {Document} doc + */ + updateMenuItems(doc) { + // When a native text-editing controller handles undo/redo (e.g. focused + // input), show generic labels and let it take over + this._updateMenuItem(doc, 'menu_undo', 'text-action-undo', 'menu-edit-undo-action', + !this.hasNativeUndo(doc) && this.getUndoAction()); + this._updateMenuItem(doc, 'menu_redo', 'text-action-redo', 'menu-edit-redo-action', + !this.hasNativeRedo(doc) && this.getRedoAction()); + }, + + _updateMenuItem(doc, id, genericMessageID, actionMessageID, action) { + let menuitem = doc.getElementById(id); + if (!menuitem) { + return; + } + if (action) { + let actionLabel = Zotero.ftl.formatValueSync( + action.action, action.actionArgs || undefined + ); + menuitem.removeAttribute('data-l10n-id'); + menuitem.setAttribute('label', Zotero.ftl.formatValueSync( + actionMessageID, { action: actionLabel } + )); + } + else { + doc.l10n.setAttributes(menuitem, genericMessageID); + } + }, + /** * Stage a change record. Must be called inside a transaction; without * a matching stageAction() in the same transaction, the record is @@ -367,9 +648,7 @@ Zotero.UndoHistory = { return; } Zotero.DB.requireTransaction(); - if (!this._pendingEntry) { - this._pendingEntry = { changes: [], action: null, actionArgs: null }; - } + this._ensurePendingEntry(); let existing = this._pendingEntry.changes.find( c => c.objectType === changeRecord.objectType && c.id === changeRecord.id); if (existing) { diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl index a5c5372542..5e1405061e 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/chrome/locale/en-US/zotero/zotero.ftl @@ -1262,6 +1262,26 @@ undo-action-merge-items = { $count -> [one] Merge Item *[other] Merge { $count } Items } +undo-action-add-annotation = { $count -> + [one] Add Annotation + *[other] Add { $count } Annotations +} +undo-action-edit-annotation = { $count -> + [one] Edit Annotation + *[other] Edit { $count } Annotations +} +undo-action-delete-annotation = { $count -> + [one] Delete Annotation + *[other] Delete { $count } Annotations +} +undo-action-convert-annotation = { $count -> + [one] Convert Annotation + *[other] Convert { $count } Annotations +} +undo-action-merge-annotations = { $count -> + [one] Merge Annotation + *[other] Merge { $count } Annotations +} menu-edit-undo-action = Undo { $action } menu-edit-redo-action = Redo { $action } diff --git a/test/tests/readerTest.js b/test/tests/readerTest.js index 36c68d5fbb..733da17d83 100644 --- a/test/tests/readerTest.js +++ b/test/tests/readerTest.js @@ -214,6 +214,192 @@ describe("Reader", function () { } } + it('should record annotation changes in the app-wide undo history', async function () { + let attachment = await importFileAttachment('test.pdf'); + let reader = await Zotero.Reader.open(attachment.itemID); + try { + await reader._initPromise; + let annotationManager = reader._internalReader._annotationManager; + annotationManager._skipAnnotationSavingDebounce = true; + Zotero.UndoHistory.clear(); + + let annotation = annotationManager.addAnnotation( + Components.utils.cloneInto({ + type: 'highlight', + color: '#ffd400', + sortIndex: '00000|003305|00000', + position: { + pageIndex: 0, + rects: [[0, 0, 100, 100]] + }, + text: 'test' + }, reader._iframeWindow) + ); + await waitForItemEvent('add'); + assert.equal(attachment.getAnnotations()[0].key, annotation.id); + assert.isTrue(Zotero.UndoHistory.canUndo()); + assert.equal( + Zotero.UndoHistory.getUndoAction().action, 'undo-action-add-annotation' + ); + + // Undoing from outside the reader should remove the annotation + assert.isTrue(await Zotero.UndoHistory.undo()); + await waitForItemEvent('delete'); + assert.lengthOf(attachment.getAnnotations(), 0); + + // Redoing should bring it back (with a new key, to avoid sync conflicts) + assert.isTrue(await Zotero.UndoHistory.redo()); + await waitForItemEvent('add'); + assert.lengthOf(attachment.getAnnotations(), 1); + + // An edit should be undoable as its own step + annotationManager.updateAnnotations( + Components.utils.cloneInto([{ + id: attachment.getAnnotations()[0].key, + text: 'test2' + }], reader._iframeWindow) + ); + await waitForItemEvent('modify'); + assert.equal( + Zotero.UndoHistory.getUndoAction().action, 'undo-action-edit-annotation' + ); + assert.isTrue(await Zotero.UndoHistory.undo()); + await waitForItemEvent('modify'); + assert.equal(attachment.getAnnotations()[0].annotationText, 'test'); + } + finally { + await cleanupReaders(reader); + } + + // A closed reader can't apply its steps anymore + assert.isFalse(Zotero.UndoHistory.canUndo()); + assert.isFalse(Zotero.UndoHistory.canRedo()); + }); + + it('should select the reader tab when undoing an annotation change from the library', async function () { + let attachment = await importFileAttachment('test.pdf'); + let reader = await Zotero.Reader.open(attachment.itemID); + try { + await reader._initPromise; + let annotationManager = reader._internalReader._annotationManager; + annotationManager._skipAnnotationSavingDebounce = true; + Zotero.UndoHistory.clear(); + + annotationManager.addAnnotation( + Components.utils.cloneInto({ + type: 'highlight', + color: '#ffd400', + sortIndex: '00000|003305|00000', + position: { + pageIndex: 0, + rects: [[0, 0, 100, 100]] + }, + text: 'test' + }, reader._iframeWindow) + ); + await waitForItemEvent('add'); + + win.Zotero_Tabs.select('zotero-pane'); + assert.isTrue(await Zotero.UndoHistory.undo()); + await waitForItemEvent('delete'); + assert.equal(win.Zotero_Tabs.selectedID, reader.tabID); + } + finally { + await cleanupReaders(reader); + } + }); + + it('should select the library tab when undoing a library change from a reader', async function () { + let reader; + try { + win.Zotero_Tabs.select('zotero-pane'); + let collection = await createDataObject('collection', { name: 'Original' }); + Zotero.UndoHistory.clear(); + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + + let attachment = await importFileAttachment('test.pdf'); + reader = await Zotero.Reader.open(attachment.itemID); + await reader._initPromise; + assert.equal(win.Zotero_Tabs.selectedID, reader.tabID); + + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.equal(collection.name, 'Original'); + assert.equal(win.Zotero_Tabs.selectedID, 'zotero-pane'); + } + finally { + await cleanupReaders(reader); + } + }); + + it('should run undo commands from the app-wide history while a reader view has focus', async function () { + let attachment = await importFileAttachment('test.pdf'); + let reader = await Zotero.Reader.open(attachment.itemID); + try { + await reader._initPromise; + let annotationManager = reader._internalReader._annotationManager; + annotationManager._skipAnnotationSavingDebounce = true; + Zotero.UndoHistory.clear(); + + annotationManager.addAnnotation( + Components.utils.cloneInto({ + type: 'highlight', + color: '#ffd400', + sortIndex: '00000|003305|00000', + position: { + pageIndex: 0, + rects: [[0, 0, 100, 100]] + }, + text: 'test' + }, reader._iframeWindow) + ); + await waitForItemEvent('add'); + + await reader.focus(); + await Zotero.Promise.delay(100); + // Focus only moves into the reader while its window is active. + // It lands in a view iframe, whose frame tree is rooted at the + // reader itself. + if (win.document.commandDispatcher.focusedWindow?.top !== reader._iframeWindow) { + Zotero.debug("Skipping test -- reader couldn't be focused"); + this.skip(); + } + + assert.isFalse(Zotero.UndoHistory.hasNativeUndo(win.document)); + // As the Edit menu and key_undo do + win.goDoCommand('cmd_undo'); + await waitForItemEvent('delete'); + assert.lengthOf(attachment.getAnnotations(), 0); + } + finally { + await cleanupReaders(reader); + } + }); + + it('should leave undo to text editing while a reader text box has focus', async function () { + let attachment = await importFileAttachment('test.pdf'); + let reader = await Zotero.Reader.open(attachment.itemID); + try { + await reader._initPromise; + await reader._internalReader._primaryView.initializedPromise; + let doc = reader._iframeWindow.document; + let pageNumberInput = doc.getElementById('pageNumber'); + pageNumberInput.focus(); + await Zotero.Promise.delay(100); + // Focus only moves into the reader while its window is active + if (win.document.commandDispatcher.focusedWindow !== reader._iframeWindow + || doc.activeElement !== pageNumberInput) { + Zotero.debug("Skipping test -- reader text box couldn't be focused"); + this.skip(); + } + + assert.isTrue(Zotero.UndoHistory.hasNativeUndo(win.document)); + } + finally { + await cleanupReaders(reader); + } + }); + it('should reopen a reader whose tab closes during a notifier transaction', async function () { let reader, reopenedReader; let title = Zotero.Promise.defer(); @@ -352,6 +538,45 @@ describe("Reader", function () { } }); + it('should name the undone action in a reader window Edit menu', async function () { + let attachment = await importFileAttachment('test.pdf'); + let reader = await Zotero.Reader.open(attachment.id, null, { openInWindow: true }); + try { + await reader._initPromise; + let annotationManager = reader._internalReader._annotationManager; + annotationManager._skipAnnotationSavingDebounce = true; + Zotero.UndoHistory.clear(); + + annotationManager.addAnnotation( + Components.utils.cloneInto({ + type: 'highlight', + color: '#ffd400', + sortIndex: '00000|003305|00000', + position: { + pageIndex: 0, + rects: [[0, 0, 100, 100]] + }, + text: 'test' + }, reader._iframeWindow) + ); + await waitForItemEvent('add'); + + let doc = reader._window.document; + Zotero.UndoHistory.updateMenuItems(doc); + let undoItem = doc.getElementById('menu_undo'); + assert.isFalse(undoItem.hasAttribute('data-l10n-id')); + assert.equal( + undoItem.getAttribute('label'), + Zotero.ftl.formatValueSync('menu-edit-undo-action', { + action: Zotero.ftl.formatValueSync('undo-action-add-annotation', { count: 1 }) + }) + ); + } + finally { + await cleanupReaders(reader); + } + }); + describe("#importFromEPUB()", function () { let bookEpubPath; // The EPUB itself let bookSdrPath; // The KOReader "sidecar" folder diff --git a/test/tests/undoHistoryTest.js b/test/tests/undoHistoryTest.js index bac0cedfb5..3f3c00563a 100644 --- a/test/tests/undoHistoryTest.js +++ b/test/tests/undoHistoryTest.js @@ -1577,6 +1577,237 @@ describe("Zotero.UndoHistory", function () { }); }); + describe("external providers", function () { + // Stands in for a reader instance: keeps its own stack of steps, reports + // it after every change, and steps it on request + function createProvider(providerID) { + let provider = { + id: providerID, + undoSteps: [], + redoSteps: [], + revealCount: 0, + _lastStepID: 0, + + register() { + Zotero.UndoHistory.registerProvider(this.id, { + libraryID: Zotero.Libraries.userLibraryID, + undo: () => this.undo(), + redo: () => this.redo(), + reveal: () => this.revealCount++ + }); + return this; + }, + + addStep(action = 'undo-action-edit-annotation', count = 1) { + let step = { + id: ++this._lastStepID, + revision: 0, + action, + actionArgs: { count } + }; + this.undoSteps.push(step); + this.redoSteps = []; + this.report(); + return step; + }, + + // A change joined into the newest step, as continued typing is + reviseNewestStep() { + this.undoSteps[this.undoSteps.length - 1].revision++; + this.redoSteps = []; + this.report(); + }, + + // An outside change invalidating everything up to and including + // the given step + invalidateThrough(step) { + this.undoSteps = this.undoSteps.slice(this.undoSteps.indexOf(step) + 1); + this.report(); + }, + + undo() { + let step = this.undoSteps.pop(); + if (!step) { + return false; + } + this.redoSteps.push(step); + this.report(); + return true; + }, + + redo() { + let step = this.redoSteps.pop(); + if (!step) { + return false; + } + this.undoSteps.push(step); + this.report(); + return true; + }, + + report() { + Zotero.UndoHistory.setProviderSteps(this.id, { + undoSteps: this.undoSteps, + redoSteps: this.redoSteps + }); + } + }; + return provider; + } + + var provider; + + beforeEach(function () { + provider = createProvider('provider-' + Zotero.Utilities.randomString()).register(); + }); + + afterEach(function () { + Zotero.UndoHistory.unregisterProvider(provider.id); + }); + + it("should undo and redo a provider step", async function () { + provider.addStep('undo-action-add-annotation'); + assert.isTrue(Zotero.UndoHistory.canUndo()); + assert.deepEqual( + Zotero.UndoHistory.getUndoAction(), + { action: 'undo-action-add-annotation', actionArgs: { count: 1 } } + ); + + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.lengthOf(provider.undoSteps, 0); + assert.lengthOf(provider.redoSteps, 1); + assert.isFalse(Zotero.UndoHistory.canUndo()); + assert.equal( + Zotero.UndoHistory.getRedoAction().action, 'undo-action-add-annotation' + ); + + assert.isTrue(await Zotero.UndoHistory.redo()); + assert.lengthOf(provider.undoSteps, 1); + assert.isTrue(Zotero.UndoHistory.canUndo()); + assert.isFalse(Zotero.UndoHistory.canRedo()); + }); + + it("should ask the provider to reveal itself after applying a step", async function () { + provider.addStep(); + + await Zotero.UndoHistory.undo(); + assert.equal(provider.revealCount, 1); + + await Zotero.UndoHistory.redo(); + assert.equal(provider.revealCount, 2); + }); + + it("should not reveal a provider that declined to step", async function () { + provider.addStep(); + provider.undoSteps = []; + + assert.isFalse(await Zotero.UndoHistory.undo()); + assert.equal(provider.revealCount, 0); + }); + + it("should interleave provider steps with captured changes", async function () { + let collection = await createDataObject('collection', { name: 'Original' }); + Zotero.UndoHistory.clear(); + + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + provider.addStep('undo-action-add-annotation'); + + // The provider step came last, so it goes first + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.lengthOf(provider.undoSteps, 0); + assert.equal(collection.name, 'Modified'); + + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.equal(collection.name, 'Original'); + }); + + it("should move a revised step back to the top of the stack", async function () { + let collection = await createDataObject('collection', { name: 'Original' }); + Zotero.UndoHistory.clear(); + + provider.addStep(); + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + // The provider's step now covers a change made after the rename + provider.reviseNewestStep(); + + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.lengthOf(provider.undoSteps, 0); + assert.equal(collection.name, 'Modified'); + }); + + it("should clear the redo stack when a provider records a new step", async function () { + let collection = await createDataObject('collection', { name: 'Original' }); + Zotero.UndoHistory.clear(); + + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + await Zotero.UndoHistory.undo(); + assert.isTrue(Zotero.UndoHistory.canRedo()); + + provider.addStep(); + assert.isFalse(Zotero.UndoHistory.canRedo()); + }); + + it("should drop entries for steps the provider no longer has", async function () { + let firstStep = provider.addStep(); + provider.addStep(); + provider.invalidateThrough(firstStep); + + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.isFalse(await Zotero.UndoHistory.undo()); + }); + + it("should not restore steps that were already discarded", async function () { + provider.addStep(); + Zotero.UndoHistory.clear(); + + // Reporting the older step again along with a new one shouldn't + // bring the cleared entry back + provider.addStep(); + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.isFalse(await Zotero.UndoHistory.undo()); + }); + + it("should discard a provider's entries when it declines to step", async function () { + let collection = await createDataObject('collection', { name: 'Original' }); + Zotero.UndoHistory.clear(); + + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + provider.addStep(); + provider.addStep(); + // Provider has lost track of its own history + provider.undoSteps = []; + + assert.isFalse(await Zotero.UndoHistory.undo()); + // The remaining provider entry is gone, but the captured change isn't + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.equal(collection.name, 'Original'); + }); + + it("should discard entries when a provider is unregistered", async function () { + provider.addStep(); + assert.isTrue(Zotero.UndoHistory.canUndo()); + + Zotero.UndoHistory.unregisterProvider(provider.id); + assert.isFalse(Zotero.UndoHistory.canUndo()); + }); + + it("should discard provider entries for an erased library", async function () { + provider.addStep(); + Zotero.UndoHistory.clearForLibrary(Zotero.Libraries.userLibraryID); + assert.isFalse(Zotero.UndoHistory.canUndo()); + }); + + it("should ignore steps from an unregistered provider", function () { + Zotero.UndoHistory.unregisterProvider(provider.id); + provider.addStep(); + assert.isFalse(Zotero.UndoHistory.canUndo()); + }); + }); + describe("step limit pref", function () { afterEach(function () { Zotero.Prefs.clear('undoHistory.steps'); From 67ed2f8d16f757f8b6306ae4501f98f48d485cfa Mon Sep 17 00:00:00 2001 From: Abe Jellinek <1770299+AbeJellinek@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:14:47 -0400 Subject: [PATCH 2/2] Fixes after review - Fix action absorbed into previous step not being undoable after history is cleared - Fix redo still showing as active after step is revised - Fix text input check including radio/checkbox inputs (and don't hardcode input types at all) --- chrome/content/zotero/xpcom/undoHistory.js | 72 ++++++++------ test/tests/readerTest.js | 83 ++++++++++++++++ test/tests/undoHistoryTest.js | 106 +++++++++++++++++++++ 3 files changed, 234 insertions(+), 27 deletions(-) diff --git a/chrome/content/zotero/xpcom/undoHistory.js b/chrome/content/zotero/xpcom/undoHistory.js index cc0aee0f1f..b9d91c3ce2 100644 --- a/chrome/content/zotero/xpcom/undoHistory.js +++ b/chrome/content/zotero/xpcom/undoHistory.js @@ -106,7 +106,7 @@ Zotero.UndoHistory = { */ registerProvider(providerID, { libraryID, undo, redo, reveal, window }) { this._providers.set(providerID, - { libraryID, undo, redo, reveal, window, lastStepID: 0 }); + { libraryID, undo, redo, reveal, window, seenSteps: new Map() }); }, /** @@ -124,12 +124,14 @@ Zotero.UndoHistory = { /** * Bring a provider's entries in line with the provider's own history. * - * Steps are ordered oldest first and identified by an id that increases + * Steps are ordered oldest first and identified by an ID that increases * monotonically, plus a revision that changes when a step absorbs a later * change (e.g., continued typing in an annotation comment). Steps the - * provider no longer has are dropped; steps newer than any we've seen are - * pushed onto the undo stack; and a step whose revision changed moves back - * to the top of the stack, since it now covers the most recent change. + * provider no longer has are dropped. A step we haven't seen, or one whose + * revision changed since we last saw it, covers a new change, so it goes + * to the top of the undo stack. A step reported with a revision we've + * already seen is left alone, so entries discarded in the meantime (e.g. + * by a clear()) aren't brought back. * * @param {String} providerID * @param {Object} history @@ -141,34 +143,36 @@ Zotero.UndoHistory = { if (!provider || !this.isEnabled()) { return; } - let stepIDs = new Set([...undoSteps, ...redoSteps].map(step => step.id)); + let steps = [...undoSteps, ...redoSteps]; + let stepIDs = new Set(steps.map(step => step.id)); this._filterEntries( entry => entry.providerID !== providerID || stepIDs.has(entry.stepID) ); - let added = false; + let changed = false; for (let step of undoSteps) { - if (step.id > provider.lastStepID) { - this._undoStack.push({ - providerID, - stepID: step.id, - revision: step.revision, - libraryID: provider.libraryID, - action: step.action, - actionArgs: step.actionArgs || null - }); - added = true; + if (provider.seenSteps.get(step.id) === step.revision) { continue; } let index = this._undoStack.findIndex( entry => entry.providerID === providerID && entry.stepID === step.id); - if (index !== -1 && this._undoStack[index].revision !== step.revision) { - let [entry] = this._undoStack.splice(index, 1); - entry.revision = step.revision; - this._undoStack.push(entry); + if (index === -1) { + this._undoStack.push({ + providerID, + stepID: step.id, + libraryID: provider.libraryID, + action: step.action, + actionArgs: step.actionArgs || null + }); } + else { + this._undoStack.push(...this._undoStack.splice(index, 1)); + } + changed = true; } - provider.lastStepID = Math.max(provider.lastStepID, ...stepIDs); - if (added) { + provider.seenSteps = new Map(steps.map(step => [step.id, step.revision])); + if (changed) { + // Any new change invalidates redo, including one absorbed into a + // step we already had an entry for this._redoStack = []; this._trimUndoStack(); } @@ -306,7 +310,7 @@ Zotero.UndoHistory = { if (!this._getProviderForWindow(focusedWindow)) { return true; } - return this._isTextBoxFocused(focusedWindow); + return this._isTextBoxFocused(focusedWindow, cmd); } let el = doc.commandDispatcher.focusedElement; if (!el) return false; @@ -314,6 +318,15 @@ Zotero.UndoHistory = { && !this._getProviderForWindow(el.contentWindow)) { return true; } + return this._elementSupportsCommand(el, cmd); + }, + + /** + * @param {Element} el + * @param {String} cmd + * @return {Boolean} + */ + _elementSupportsCommand(el, cmd) { let controllers; try { controllers = el.controllers; @@ -357,12 +370,17 @@ Zotero.UndoHistory = { * editing owns undo/redo for it * * @param {Window} win + * @param {String} cmd * @return {Boolean} */ - _isTextBoxFocused(win) { + _isTextBoxFocused(win, cmd) { let el = win.document.activeElement; - return !!el - && (el.isContentEditable || ['input', 'textarea'].includes(el.localName)); + if (!el) { + return false; + } + // A contenteditable's undo/redo is handled by an editing controller on + // the window rather than one of its own + return el.isContentEditable || this._elementSupportsCommand(el, cmd); }, /** diff --git a/test/tests/readerTest.js b/test/tests/readerTest.js index 733da17d83..fe024a524d 100644 --- a/test/tests/readerTest.js +++ b/test/tests/readerTest.js @@ -276,6 +276,58 @@ describe("Reader", function () { assert.isFalse(Zotero.UndoHistory.canRedo()); }); + it('should keep recording annotation edits after the undo history is cleared', async function () { + let attachment = await importFileAttachment('test.pdf'); + let reader = await Zotero.Reader.open(attachment.itemID); + try { + await reader._initPromise; + let annotationManager = reader._internalReader._annotationManager; + annotationManager._skipAnnotationSavingDebounce = true; + Zotero.UndoHistory.clear(); + + let annotation = annotationManager.addAnnotation( + Components.utils.cloneInto({ + type: 'highlight', + color: '#ffd400', + sortIndex: '00000|003305|00000', + position: { + pageIndex: 0, + rects: [[0, 0, 100, 100]] + }, + text: 'test' + }, reader._iframeWindow) + ); + await waitForItemEvent('add'); + + let editComment = async (comment) => { + annotationManager.updateAnnotations( + Components.utils.cloneInto([{ id: annotation.id, comment }], reader._iframeWindow) + ); + await waitForItemEvent('modify'); + }; + // The second edit is joined into the first one's history step, + // as continued typing in a comment is + await editComment('a'); + await editComment('ab'); + + // A sync applying remote changes discards the history + Zotero.UndoHistory.clear(); + assert.isFalse(Zotero.UndoHistory.canUndo()); + + // Typing in the same comment is joined into that step again, but + // it's still a change we haven't recorded, so it has to be undoable + await editComment('abc'); + assert.isTrue(Zotero.UndoHistory.canUndo()); + assert.isTrue(await Zotero.UndoHistory.undo()); + await waitForItemEvent('modify'); + // The reader's step reverts the last change joined into it + assert.equal(attachment.getAnnotations()[0].annotationComment, 'ab'); + } + finally { + await cleanupReaders(reader); + } + }); + it('should select the reader tab when undoing an annotation change from the library', async function () { let attachment = await importFileAttachment('test.pdf'); let reader = await Zotero.Reader.open(attachment.itemID); @@ -400,6 +452,37 @@ describe("Reader", function () { } }); + it('should not leave undo to text editing while a reader checkbox has focus', async function () { + let attachment = await importFileAttachment('test.pdf'); + let reader = await Zotero.Reader.open(attachment.itemID); + try { + await reader._initPromise; + await reader._internalReader._primaryView.initializedPromise; + reader._internalReader.toggleFindPopup( + Components.utils.cloneInto({ open: true }, reader._iframeWindow) + ); + let doc = reader._iframeWindow.document; + // The find popup focuses its own text box when it opens + let checkbox = await waitForCallback(() => doc.getElementById('highlight-all')); + await Zotero.Promise.delay(200); + checkbox.focus(); + await Zotero.Promise.delay(100); + // Focus only moves into the reader while its window is active + if (win.document.commandDispatcher.focusedWindow !== reader._iframeWindow + || doc.activeElement !== checkbox) { + Zotero.debug("Skipping test -- reader checkbox couldn't be focused"); + this.skip(); + } + + // A checkbox has no text editing component, so there's nothing + // for native text editing to undo + assert.isFalse(Zotero.UndoHistory.hasNativeUndo(win.document)); + } + finally { + await cleanupReaders(reader); + } + }); + it('should reopen a reader whose tab closes during a notifier transaction', async function () { let reader, reopenedReader; let title = Zotero.Promise.defer(); diff --git a/test/tests/undoHistoryTest.js b/test/tests/undoHistoryTest.js index 3f3c00563a..8fccf938c9 100644 --- a/test/tests/undoHistoryTest.js +++ b/test/tests/undoHistoryTest.js @@ -1737,6 +1737,24 @@ describe("Zotero.UndoHistory", function () { assert.equal(collection.name, 'Modified'); }); + it("should record a change absorbed into a step after history was cleared", async function () { + provider.addStep(); + provider.addStep(); + // As a sync that applied remote changes does + Zotero.UndoHistory.clear(); + assert.isFalse(Zotero.UndoHistory.canUndo()); + + // A change joined into a step we've already seen is still a change + // we haven't recorded, so it has to become undoable + provider.reviseNewestStep(); + assert.isTrue(Zotero.UndoHistory.canUndo()); + assert.isTrue(await Zotero.UndoHistory.undo()); + assert.lengthOf(provider.undoSteps, 1); + + // The step from before the clear stays discarded + assert.isFalse(await Zotero.UndoHistory.undo()); + }); + it("should clear the redo stack when a provider records a new step", async function () { let collection = await createDataObject('collection', { name: 'Original' }); Zotero.UndoHistory.clear(); @@ -1750,6 +1768,24 @@ describe("Zotero.UndoHistory", function () { assert.isFalse(Zotero.UndoHistory.canRedo()); }); + it("should clear the redo stack when a provider revises a step", async function () { + let collection = await createDataObject('collection', { name: 'Original' }); + Zotero.UndoHistory.clear(); + + provider.addStep(); + provider.addStep(); + collection.name = 'Modified'; + await collection.saveTx({ undoAction: 'undo-action-rename-collection' }); + await Zotero.UndoHistory.undo(); + assert.equal(collection.name, 'Original'); + assert.isTrue(Zotero.UndoHistory.canRedo()); + + // The provider's newest step now covers a further change, so + // redoing the rename over it is no longer valid + provider.reviseNewestStep(); + assert.isFalse(Zotero.UndoHistory.canRedo()); + }); + it("should drop entries for steps the provider no longer has", async function () { let firstStep = provider.addStep(); provider.addStep(); @@ -1808,6 +1844,76 @@ describe("Zotero.UndoHistory", function () { }); }); + describe("deferring to native text editing", function () { + var providerID, htmlDoc; + + // A provider frame with the given element focused within it + function createFrame(activeElement) { + let frame = { + document: { activeElement }, + get parent() { + return frame; + } + }; + Zotero.UndoHistory.registerProvider(providerID, { + libraryID: Zotero.Libraries.userLibraryID, + undo: () => true, + redo: () => true, + window: frame + }); + return frame; + } + + // A main window document with focus inside the given frame + function createDocument(focusedWindow) { + return { + defaultView: {}, + commandDispatcher: { focusedWindow, focusedElement: null } + }; + } + + function createInput(type) { + let input = htmlDoc.createElement('input'); + if (type) { + input.type = type; + } + return input; + } + + before(function () { + htmlDoc = new DOMParser().parseFromString( + '', 'text/html'); + }); + + beforeEach(function () { + providerID = 'provider-' + Zotero.Utilities.randomString(); + }); + + afterEach(function () { + Zotero.UndoHistory.unregisterProvider(providerID); + }); + + function assertDefers(el, shouldDefer) { + let doc = createDocument(createFrame(el)); + let desc = el.localName === 'input' ? `input[type=${el.type}]` : el.localName; + assert.equal(Zotero.UndoHistory.hasNativeUndo(doc), shouldDefer, desc); + assert.equal(Zotero.UndoHistory.hasNativeRedo(doc), shouldDefer, desc); + } + + it("should defer for a focused text box", function () { + for (let type of ['', 'text', 'search', 'password', 'number']) { + assertDefers(createInput(type), true); + } + assertDefers(htmlDoc.createElement('textarea'), true); + }); + + it("should not defer for a focused input that holds no text", function () { + for (let type of ['checkbox', 'radio', 'button', 'range', 'file']) { + assertDefers(createInput(type), false); + } + }); + }); + describe("step limit pref", function () { afterEach(function () { Zotero.Prefs.clear('undoHistory.steps');