This commit is contained in:
Abe Jellinek 2026-08-25 14:04:14 -04:00 committed by GitHub
commit de4d7877d2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 1071 additions and 61 deletions

View file

@ -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
*/

View file

@ -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);
@ -2079,6 +2147,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);
@ -2226,6 +2304,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;
}
@ -2257,6 +2341,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
@ -2296,6 +2384,7 @@ class ReaderWindow extends ReaderInstance {
return;
}
this._window.goUpdateGlobalEditMenuItems(true);
Zotero.UndoHistory.updateMenuItems(this._window.document);
this.onUpdateCustomMenus(event, 'edit', popup);
}
@ -2886,7 +2975,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)

View file

@ -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,162 @@ 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<boolean>
* @param {Function} options.redo () => boolean | Promise<boolean>
* @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, seenSteps: new Map() });
},
/**
* 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. 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
* @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 steps = [...undoSteps, ...redoSteps];
let stepIDs = new Set(steps.map(step => step.id));
this._filterEntries(
entry => entry.providerID !== providerID || stepIDs.has(entry.stepID)
);
let changed = false;
for (let step of undoSteps) {
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.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.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();
}
},
_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 +302,31 @@ 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, cmd);
}
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;
}
return this._elementSupportsCommand(el, cmd);
},
/**
* @param {Element} el
* @param {String} cmd
* @return {Boolean}
*/
_elementSupportsCommand(el, cmd) {
let controllers;
try {
controllers = el.controllers;
@ -174,6 +344,45 @@ 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
* @param {String} cmd
* @return {Boolean}
*/
_isTextBoxFocused(win, cmd) {
let el = win.document.activeElement;
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);
},
/**
* Undo the most recent change entry. Serialized through a queue.
*
@ -239,6 +448,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 +479,7 @@ Zotero.UndoHistory = {
return false;
}
this[toStack].push(entry);
this._revealEntry(entry);
return true;
}
catch (e) {
@ -281,6 +494,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<Boolean>} -- 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 +543,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 +552,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 +588,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 +615,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 +666,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) {

View file

@ -1293,6 +1293,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 }

View file

@ -214,6 +214,275 @@ 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 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);
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 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();
@ -352,6 +621,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

View file

@ -1577,6 +1577,343 @@ 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 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();
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 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();
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("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(
'<!DOCTYPE html><html><body></body></html>', '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');