Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty Error setting field #1757

Closed
grimes2 opened this issue Aug 17, 2016 · 11 comments
Closed

Empty Error setting field #1757

grimes2 opened this issue Aug 17, 2016 · 11 comments
Labels
bug Confirmed bugs or reports that are very likely to be bugs
Milestone

Comments

@grimes2
Copy link
Contributor

grimes2 commented Aug 17, 2016

JabRef version 3.5 on Windows 10

Steps to reproduce:

  1. Insert [{a in article's title field
  2. Save database
  3. Error setting field empty
  4. forced: Kill JabRef process

jabref - c__users_lange_downloads_test bib_ bibtex mode _2016-08-17_01-54-20
(covering another message box: Error: Braces don't match)

@mlep
Copy link
Contributor

mlep commented Aug 17, 2016

Thank you for your report.
I have tried to reproduce it with the current development version (on my linux computer): It shows the message box "Error:Braces don't match", but I could click on "OK" and continue editing the file. So, it seems to work.
Please, could you try with the dev version on your windows 10?
You can find the development version here: http://builds.jabref.org/master/

@grimes2
Copy link
Contributor Author

grimes2 commented Aug 17, 2016

Same bug with the development version. Please try to save database repeatedly, because the bug is not always reproducible.. [{a is the only entry in title field. (I think the problem is, that two braces don't match, so two exeptions are thrown)

@oscargus
Copy link
Contributor

I can reproduce it. You (or at least I) have to click on the save icon, pressing Ctrl+s doesn't trigger the crash.

@oscargus oscargus added the bug Confirmed bugs or reports that are very likely to be bugs label Aug 17, 2016
@oscargus
Copy link
Contributor

Interesting:

  • Ctrl + s => two error dialogs, continue working
  • File -> Save database => one error dialog, continue working
  • Save button in toolbar => crash with blank (after a very short time) error dialog

My guess is that this may be related to some old code where we catch more or less any exception and ignore it...

@oscargus
Copy link
Contributor

I can also add that it doesn't seem related to any EDT issue (although there is one solved in #1725 that will be triggered later in the save process, but JabRef freezes before getting there).

@tobiasdiez tobiasdiez added this to the v3.6 milestone Aug 17, 2016
@oscargus
Copy link
Contributor

I looked a bit more intro this and what happens is that for ctrl+s the first dialog shows up and then is the save action not performed until "OK" is clicked. Then during the actual save, the next dialog shows.

When pressing the toolbar button, the save action starts despite the first dialog not being confirmed. Then, after a while, a second dialog shows up and then it freeze. It seems related to the spin library somehow. Looking at BasePanel.runCommand() it is the following line that freezes the program:

                wrk.run(); // Runs the potentially time-consuming action
                // without freezing the GUI. The magic is that THIS line
                // of execution will not continue until run() is finished.

probably because both dialogs are up at the same time (the first one cannot be clicked during debug).

@oscargus
Copy link
Contributor

OK, I think I know what the problem is, but I do not really know how to solve it.

  • Ctrl-s => trigger SaveDatabaseAction in EntryEditor which runs updateField that trigger the first dialog, waits for it to press OK and then invokes the actual save action using BasePanel.runCommand() that invokes exporter SaveDatabaseAction which blocks the ui in the end of init in the three-step init-run-update process and unblocks it at the end of update. During the run another call to updateField is done which trigger the second dialog (the same code as in updateField at least). As run is done outside of the EDT, the EDT can take care of the dialog.
  • Save menu => Only the call to BasePanel.runCommand()
  • Toolbar button => as for save menu, but there is also a FocusLost event triggered which leads to the first dialog. Once the dialog is shown (no need to click it), the EDT continue running the save action, blocking the input and showing the next dialog, which is when things freezes as the EDT is busy waiting for the first dialog (or something)...

Any ideas how to solve it?

@grimes2
Copy link
Contributor Author

grimes2 commented Aug 24, 2016

AFAIK it's recommended to use for this cases methods or statements that are synchronized.

@koppor koppor modified the milestones: v3.7, v3.6 Aug 25, 2016
@oscargus
Copy link
Contributor

Fixed in #1863.

@grimes2
Copy link
Contributor Author

grimes2 commented Aug 27, 2016

For Ctrl-S and File > Save database the invalidBackgroundColor (red) is only flashing up for a short period of time. This is, because the FieldEditorFocusListener sets the ValidBackgroundColor (white) on focusLost. I think this is not necessary, because the EntryEditor sets the ValidBackgroundColor already. Should I fix that?

@oscargus
Copy link
Contributor

If you think you have a solution it would be excellent! Please also see #1866 in case they are related somehow. I guess we really need to figure out some good logic for these things (how FocusLost, Save, selecting a new entry etc interact), but since I do not really see a "total rewrite" solution anything that improves it is welcome.

Btw, I learned today that if the JabRefFrame is used as parent for JOptionPane it becomes modal, so everything (most?) else is stopped and waits. Just thinking when discussing the issue solved here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs
Projects
None yet
Development

No branches or pull requests

5 participants