This was working until recently. I wonder what happend to it.

I would suggest adding a text field to the reproducer and try it out

ctrl + y still produces exception when I play around with multiple crtl+z and ctrl+y in comment field

javax.swing.undo.CannotRedoException
	at java.desktop/javax.swing.undo.UndoManager.tryUndoOrRedo(UndoManager.java:469)
	at java.desktop/javax.swing.undo.UndoManager.redo(UndoManager.java:453)
	at org.jabref@100.0.0/org.jabref.gui.undo.CountingUndoManager.redo(CountingUndoManager.java:37)
	at org.jabref@100.0.0/org.jabref.gui.undo.RedoAction.execute(RedoAction.java:21)
	at org.jabref@100.0.0/org.jabref.gui.actions.JabRefAction.lambda$new$1(JabRefAction.java:25)
	at org.controlsfx.controls@11.2.1/org.controlsfx.control.action.Action.handle(Action.java:423)
	at org.controlsfx.controls@11.2.1/org.controlsfx.control.action.Action.handle(Action.java:64)
	at javafx.base@22.0.1/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86)
	at javafx.base@22.0.1/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
	at javafx.base@22.0.1/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
	at javafx.base@22.0.1/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at javafx.base@22.0.1/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@22.0.1/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at javafx.base@22.0.1/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:49)
	at javafx.base@22.0.1/javafx.event.Event.fireEvent(Event.java:198)
	at javafx.controls@22.0.1/javafx.scene.control.MenuItem.fire(MenuItem.java:459)
	at javafx.controls@22.0.1/com.sun.javafx.scene.control.GlobalMenuAdapter.lambda$bindMenuItemProperties$2(GlobalMenuAdapter.java:150)
	at javafx.base@22.0.1/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86)
	at javafx.base@22.0.1/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
	at javafx.base@22.0.1/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
	at javafx.base@22.0.1/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at javafx.base@22.0.1/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@22.0.1/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at javafx.base@22.0.1/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
	at javafx.base@22.0.1/javafx.event.Event.fireEvent(Event.java:198)
	at javafx.controls@22.0.1/javafx.scene.control.MenuItem.fire(MenuItem.java:459)
	at javafx.graphics@22.0.1/com.sun.javafx.tk.quantum.GlassSystemMenu$1.action(GlassSystemMenu.java:237)

ctrl + y still produces exception when I play around with multiple crtl+z and ctrl+y in comment field

This is not the original issue. The original issue is an NPE. - I will look into that nevertheless.

Update: The thing triggered according to the exception is a menu item; not through the text field.

javax.swing.undo.CannotRedoException
at java.desktop/javax.swing.undo.UndoManager.tryUndoOrRedo(UndoManager.java:469)

Nice find! Was because of my OO refactoring. I did not copy the full branch for RedoAction. 😅

Follow up:

• Have menu items disabled if there is no undo / redo
• Implement undo/redo for "Clear, Change Case, Normalize"

Please do not verschlimmbesser the undoredo stuff anymore. Small refactorings ok, but the whole concept of undo/redo has to be redone.

Please do not verschlimmbesser the undoredo stuff anymore. Small refactorings ok, but the whole concept of undo/redo has to be redone.

Aim for this PR: Get it working for users. Do as less changes as possible. Keep changes local. If new code is required, add clean code. Do not add hacky code. -- Therefore, I needed to make UndoRedoAction abstract and exchange the hacky if to real classes. (I don't find the name of the resolved anti pattern here)

You can probably remove the now abstract undoredo class anyway and make the undo and the redo class directly extend simplecommand.

You can probably remove the now abstract undoredo class anyway and make the undo and the redo class directly extend simplecommand.

Done in 765e643 (#11421). I think, this code duplication is OK...

• Have menu items disabled if there is no undo / redo

Discussed with @calixtus: Very hard. -- I also tried it locally, but the dependency of tab -> undomanger -> canUndo drives me crazy. I was searching for Bindings.andThen. Chain of bindings. But I did not find any solution.

* Implement undo/redo for "Clear, Change Case, Normalize"

I checked in this branch. Works.

@LoayGhreeb I reply to your comment #11420 (comment) here to be able to track the progress w.r.t. to the code commits. (The commits and the comments are sorted by time in a pull request. Thus, one sees, whether a comment was made before a commit or after one).

Using the menu bar (Edit -> Undo) to undo changes works without any exceptions.

This is good 😅

Another issue, which may or may not be related, is that when updating a field using the context menu (Clear, Change Case, Normalize) and using Ctrl+Z to undo this change, nothing happens and no exception occurs.

I tested it with the latst commit here. Works.

However, when using "Protect Selection" from the context menu, this change is undoable but with the exception.

Works here without exception.

Maybe, you can re-check? 😅

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

Maybe, you can re-check? 😅

I tried it, and it works without exceptions. However, can we do some checks before doing the undo/redo to see if the change is related to the focused field and selected entry or not?
Because now, if I add/remove an entry, pressing Ctrl + Z/Y in the text field will remove/restore the entry again.

Maybe, you can re-check? 😅
I tried it, and it works without exceptions. However, can we do some checks before doing the undo/redo to see if the change is related to the focused field and selected entry or not?

Why? I was thinking that there should be one line of redo/undo changes and not some local textfield ones and some "global" ones not realted to the textfield ones?

I think, the behavior of the last release was that an undo of a text field (JavaFX behavior) was recorded as new change event in JabRef's global undo/redo. And then pressing undo from the menu lead to strange effects.

Now, there is only one global undo/redo chain. For sure, if one changes a text field, does somewhere else somethings, than goes back to the text field, types a letter, presses ctrl+Z twice, then the "somethings" are undone.

Because now, if I add/remove an entry, pressing Ctrl + Z/Y in the text field will remove/restore the entry again.

This is the effect of a single global undo/redo stack.

Alternatively, we could "jump" in the undo chain and search for the last action regarding the text field and undo/redo that. -- I would stay back from using JavaFX textfield's local undo/redo (put aside for now that there is an NPE and we are in the need of fix JavaFX again).

I'm okay with the current approach.

This is the effect of a single global undo/redo stack.

Alternatively, we could "jump" in the undo chain and search for the last action regarding the text field and undo/redo that. -- I would stay back from using JavaFX textfield's local undo/redo (put aside for now that there is an NPE and we are in the need of fix JavaFX again).

This can be resolved maybe by making undo actions in an event stream mergeable. So related edits (like entering a character) can be merged to larger undoables und be undone and redone as a compund (like now)

This is the effect of a single global undo/redo stack.

Alternatively, we could "jump" in the undo chain and search for the last action regarding the text field and undo/redo that. -- I would stay back from using JavaFX textfield's local undo/redo (put aside for now that there is an NPE and we are in the need of fix JavaFX again).

This can be resolved maybe by making undo actions in an event stream mergeable. So related edits (like entering a character) can be merged to larger undoables und be undone and redone as a compund (like now)

In addition, I meant:

1. Type a in field F1.
2. Type b in field F2.
3. Type c in Field F1.
4. Press Ctrl+Z twice in Field F1.

Loay expects F1 being empty.

Current behavior: F1 contains a and F2 contains nothing.

i see.
i would expect the current behaviour. if you are jumping between fields, then every change has its own compund.

OK, Loay is asking for "Objektbezogenes Undo". The text there does not fully explain how it works with hierarchical objects. It also does not link real-world examples (which makes it a "pattern candidate")

if you are jumping between fields, then every change has its own compund.

We need to craft this. As you outlined. - But maybe we should only append if it is the same operation: add / delete. Replace should be in a new thing.

Meaning:

• (a, ab) + (ab, abc) ->(a, abc) // both changes were additions, merge second into first
• (abcd, abcde) + (abcde, abcd) -> (abcd, abcde), (abcde, abcd) // different actions, keep both, no merge