JabRef / jabref

Graphical Java application for managing BibTeX and biblatex (.bib) databases
https://devdocs.jabref.org
MIT License
3.55k stars 2.48k forks source link

undo of LaTeX -> UTF conversion fails #2973

Closed jonasstein closed 7 years ago

jonasstein commented 7 years ago

How to reproduce:

add doi 10.1088/0953-8984/27/44/446001

which provides

@Article{Stein2015b,
  author    = {J Stein and M Baum and S Holbein and T Cronert and V Hutanu and A C Komarek and M Braden},
  title     = {Control of multiferroic domains by external electric fields in {TbMnO}3},
  journal   = {Journal of Physics: Condensed Matter},
  year      = {2015},
  volume    = {27},
  number    = {44},
  month     = {oct},
  pages     = {446001},
  doi       = {10.1088/0953-8984/27/44/446001},
  publisher = {{IOP} Publishing},
}

Now click with the right mouse button on the title and convert LaTeX -> UTF

Finally try undo. It does not work. The conversion function "forgot" to save before changing. It should be undoable.

JabRef 4.0.0-dev--snapshot--2017-07-03--master--6897b10fb
Linux 4.4.0-78-generic amd64 
Java 1.8.0_131
lenhard commented 7 years ago

It seems that the undo/redo operations of all right-click operations in the entry editor got lost when rewriting the entry editor with JavaFX.

@tobiasdiez The undo is working for the field contents themselves. Could you give a hint on how the undo is supported there so that we can replicate it with the right-click menu?

tobiasdiez commented 7 years ago

I have no idea why undo works for the fields :-) Maybe there is global listener for field changes that adds them automatically as undo operations? Up to now no JavaFX code works with the undo/redo mechanism (the corresponding code is still commented-out in the groups interface). The reason was/is that I want to replace our undo facilities with a library (maybe https://github.com/TomasMikula/UndoFX).

lenhard commented 7 years ago

I've looked into this for a while, but without getting something to work unfortunately. The undo works for the fields right now, because the JavaFX TextArea comes with a default undo implementation, but if you change focus after the edit, the undo is lost.

This is bad. Undo/Redo is practically non-existent in the entry editor.

And after some more thought: I think we need to figure out how to use the existing undo manager with JavaFX.

Siedlerchr commented 7 years ago

In fact, the RichTextFX already makes use of the UndoFX library mentioned above. http://programtalk.com/vs/RichTextFX/richtextfx/src/main/java/org/fxmisc/richtext/model/UndoActions.java/ So that is why it magically works. Seems like we just need to couple this with ours.

lenhard commented 7 years ago

Unfortunately, it is not that easy. We still have a swing-based undo manager in place that can undo things like adding a new entry into the main table. Either we integrate that with a new undo library, or we rewrite everything undo-related with the new library. Any way, this won't be straight-forward.

lenhard commented 7 years ago

So this is a blocker for a 4.0 release. @tobiasdiez Do you see any chance of getting this done in the not-too-far-off future?

Siedlerchr commented 7 years ago

At the moment I am getting an exception: Could this be related to the markDirty thing?

12:35:28.967 [AWT-EventQueue-0] WARN  org.jabref.gui.BasePanel - Nothing to undo
javax.swing.undo.CannotUndoException: null
    at javax.swing.undo.UndoManager.undo(UndoManager.java:411) ~[?:1.8.0_131]
    at org.jabref.gui.undo.CountingUndoManager.undo(CountingUndoManager.java:30) ~[bin/:?]
    at org.jabref.gui.BasePanel$UndoAction.action(BasePanel.java:2169) [bin/:?]
    at org.jabref.gui.BasePanel.runCommand(BasePanel.java:1027) [bin/:?]
    at org.jabref.gui.entryeditor.EntryEditor$UndoAction.actionPerformed(EntryEditor.java:877) [bin/:?]
    at javax.swing.SwingUtilities.notifyAction(SwingUtilities.java:1663) [?:1.8.0_131]
    at javax.swing.JComponent.processKeyBinding(JComponent.java:2882) [?:1.8.0_131]
    at javax.swing.KeyboardManager.fireBinding(KeyboardManager.java:307) [?:1.8.0_131]
    at javax.swing.KeyboardManager.fireKeyboardAction(KeyboardManager.java:250) [?:1.8.0_131]
    at javax.swing.JComponent.processKeyBindingsForAllComponents(JComponent.java:2974) [?:1.8.0_131]
    at javax.swing.JComponent.processKeyBindings(JComponent.java:2966) [?:1.8.0_131]
    at javax.swing.JComponent.processKeyEvent(JComponent.java:2845) [?:1.8.0_131]
    at javafx.embed.swing.JFXPanel.processKeyEvent(JFXPanel.java:463) [jfxrt.jar:?]
    at java.awt.Component.processEvent(Component.java:6310) [?:1.8.0_131]
    at java.awt.Container.processEvent(Container.java:2236) [?:1.8.0_131]
    at java.awt.Component.dispatchEventImpl(Component.java:4889) [?:1.8.0_131]
    at java.awt.Container.dispatchEventImpl(Container.java:2294) [?:1.8.0_131]
    at java.awt.Component.dispatchEvent(Component.java:4711) [?:1.8.0_131]
    at java.awt.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1954) [?:1.8.0_131]
    at java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(DefaultKeyboardFocusManager.java:806) [?:1.8.0_131]
    at java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:1074) [?:1.8.0_131]
    at java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:945) [?:1.8.0_131]
    at java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:771) [?:1.8.0_131]
    at java.awt.Component.dispatchEventImpl(Component.java:4760) [?:1.8.0_131]
    at java.awt.Container.dispatchEventImpl(Container.java:2294) [?:1.8.0_131]
    at java.awt.Window.dispatchEventImpl(Window.java:2746) [?:1.8.0_131]
    at java.awt.Component.dispatchEvent(Component.java:4711) [?:1.8.0_131]
    at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758) [?:1.8.0_131]
    at java.awt.EventQueue.access$500(EventQueue.java:97) [?:1.8.0_131]
    at java.awt.EventQueue$3.run(EventQueue.java:709) [?:1.8.0_131]
    at java.awt.EventQueue$3.run(EventQueue.java:703) [?:1.8.0_131]
    at java.security.AccessController.doPrivileged(Native Method) ~[?:1.8.0_131]
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:80) [?:1.8.0_131]
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:90) [?:1.8.0_131]
    at java.awt.EventQueue$4.run(EventQueue.java:731) [?:1.8.0_131]
    at java.awt.EventQueue$4.run(EventQueue.java:729) [?:1.8.0_131]
    at java.security.AccessController.doPrivileged(Native Method) ~[?:1.8.0_131]
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:80) [?:1.8.0_131]
    at java.awt.EventQueue.dispatchEvent(EventQueue.java:728) [?:1.8.0_131]
    at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201) [?:1.8.0_131]
    at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116) [?:1.8.0_131]
    at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105) [?:1.8.0_131]
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101) [?:1.8.0_131]
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93) [?:1.8.0_131]
    at java.awt.EventDispatchThread.run(EventDispatchThread.java:82) [?:1.8.0_131]
lenhard commented 7 years ago

No, it's not related to the dirty-marking, but to the undo facilities. It happens when you press undo, but there are no undo event registered in the manager (and since the entry editor does not produce undo events at the moment, this is highly likely)

tobiasdiez commented 7 years ago

This should be fixed in the latest development version. Could you please check the build from http://builds.jabref.org/master/. Thanks!

I just applied a quick and a bit hacky fix. We should try to replace the undo framework nonetheless.