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

Non valid number as font size results in an uncaught exception. #7415

Closed
1 task done
Landi29 opened this issue Feb 2, 2021 · 7 comments · Fixed by #7438
Closed
1 task done

Non valid number as font size results in an uncaught exception. #7415

Landi29 opened this issue Feb 2, 2021 · 7 comments · Fixed by #7438
Labels
bug Confirmed bugs or reports that are very likely to be bugs good first issue An issue intended for project-newcomers. Varies in difficulty. preferences

Comments

@Landi29
Copy link
Contributor

Landi29 commented Feb 2, 2021

Latest JabRef version build on Ubuntu 20.04.1 LTS.

If you enter a non valid value such as a character to the font size spinner under the appearances tab in the preference dialog and press enter, 2 exception dialogs will appear.
Steps to reproduce the behavior:

  1. Open JabRef.
  2. Click on Options
  3. Click on Preferences
  4. Click on Appearance.
  5. In the "size" box, enter a char such as a.
  6. Press the Enter key. Two uncaught exception dialogs will appear.
  7. Pressing on the preferences dialog after pressing ok to the two exception dialogs, a third exception dialog will appear.
Log File
java.lang.NumberFormatException: For input string: "a"
  at java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:68)
  at java.base/java.lang.Integer.parseInt(Integer.java:652)
  at java.base/java.lang.Integer.valueOf(Integer.java:983)
  at javafx.base/javafx.util.converter.IntegerStringConverter.fromString(IntegerStringConverter.java:49)
  at javafx.base/javafx.util.converter.IntegerStringConverter.fromString(IntegerStringConverter.java:35)
  at javafx.controls/javafx.scene.control.Spinner.commitValue(Spinner.java:460)
  at javafx.controls/javafx.scene.control.Spinner.lambda$new$3(Spinner.java:168)
  at javafx.base/com.sun.javafx.binding.ExpressionHelper$Generic.fireValueChangedEvent(ExpressionHelper.java:348)
  at javafx.base/com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:80)
  at javafx.base/javafx.beans.property.ReadOnlyBooleanPropertyBase.fireValueChangedEvent(ReadOnlyBooleanPropertyBase.java:72)
  at javafx.graphics/javafx.scene.Node$FocusedProperty.notifyListeners(Node.java:8155)
  at javafx.graphics/javafx.scene.Node.setFocused(Node.java:8208)
  at javafx.graphics/javafx.scene.Scene$KeyHandler.setWindowFocused(Scene.java:4032)
  at javafx.graphics/javafx.scene.Scene$KeyHandler.lambda$new$0(Scene.java:4054)
  at javafx.base/com.sun.javafx.binding.ExpressionHelper$SingleInvalidation.fireValueChangedEvent(ExpressionHelper.java:136)
  at javafx.base/com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:80)
  at javafx.base/javafx.beans.property.ReadOnlyBooleanPropertyBase.fireValueChangedEvent(ReadOnlyBooleanPropertyBase.java:72)
  at javafx.base/javafx.beans.property.ReadOnlyBooleanWrapper.fireValueChangedEvent(ReadOnlyBooleanWrapper.java:103)
  at javafx.base/javafx.beans.property.BooleanPropertyBase.markInvalid(BooleanPropertyBase.java:111)
  at javafx.base/javafx.beans.property.BooleanPropertyBase.set(BooleanPropertyBase.java:145)
  at javafx.graphics/javafx.stage.Window.setFocused(Window.java:674)
  at javafx.graphics/javafx.stage.Window$1.setFocused(Window.java:149)
  at javafx.graphics/com.sun.javafx.stage.WindowHelper.setFocused(WindowHelper.java:112)
  at javafx.graphics/com.sun.javafx.stage.WindowPeerListener.changedFocused(WindowPeerListener.java:64)
  at javafx.graphics/com.sun.javafx.tk.quantum.GlassWindowEventHandler.run(GlassWindowEventHandler.java:126)
  at javafx.graphics/com.sun.javafx.tk.quantum.GlassWindowEventHandler.run(GlassWindowEventHandler.java:40)
  at java.base/java.security.AccessController.doPrivileged(AccessController.java:391)
  at javafx.graphics/com.sun.javafx.tk.quantum.GlassWindowEventHandler.lambda$handleWindowEvent$4(GlassWindowEventHandler.java:176)
  at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit.runWithoutRenderLock(QuantumToolkit.java:412)
  at javafx.graphics/com.sun.javafx.tk.quantum.GlassWindowEventHandler.handleWindowEvent(GlassWindowEventHandler.java:174)
  at javafx.graphics/com.sun.glass.ui.Window.handleWindowEvent(Window.java:1351)
  at javafx.graphics/com.sun.glass.ui.Window.notifyFocus(Window.java:1330)
  at javafx.graphics/com.sun.glass.ui.gtk.GtkApplication.enterNestedEventLoopImpl(Native Method)
  at javafx.graphics/com.sun.glass.ui.gtk.GtkApplication._enterNestedEventLoop(GtkApplication.java:347)
  at javafx.graphics/com.sun.glass.ui.Application.enterNestedEventLoop(Application.java:509)
  at javafx.graphics/com.sun.glass.ui.EventLoop.enter(EventLoop.java:107)
  at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit.enterNestedEventLoop(QuantumToolkit.java:635)
  at javafx.graphics/javafx.stage.Stage.showAndWait(Stage.java:465)
  at javafx.controls/javafx.scene.control.HeavyweightDialog.showAndWait(HeavyweightDialog.java:162)
  at javafx.controls/javafx.scene.control.Dialog.showAndWait(Dialog.java:346)
  at org.jabref@100.0.0/org.jabref.gui.JabRefDialogService.showErrorDialogAndWait(JabRefDialogService.java:192)
  at org.jabref@100.0.0/org.jabref.gui.FallbackExceptionHandler.lambda$uncaughtException$0(FallbackExceptionHandler.java:26)
  at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:428)
  at java.base/java.security.AccessController.doPrivileged(AccessController.java:391)
  at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:427)
  at javafx.graphics/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:96)
  at javafx.graphics/com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
  at javafx.graphics/com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$11(GtkApplication.java:277)
  at java.base/java.lang.Thread.run(Thread.java:832)

@Siedlerchr Siedlerchr added preferences bug Confirmed bugs or reports that are very likely to be bugs labels Feb 2, 2021
@Siedlerchr
Copy link
Member

Interesting, I thought that a Spinner was already restricted to numbers, but turns out it's not https://stackoverflow.com/a/36749659
spinner.getEditor() returns the textfield

@Siedlerchr Siedlerchr added the good first issue An issue intended for project-newcomers. Varies in difficulty. label Feb 2, 2021
@Landi29
Copy link
Contributor Author

Landi29 commented Feb 2, 2021

So did I at first, but you can specify the datatype of the Spinner in the declaration. As it appears in the log, an uncaughtException is raised because the thread hits an exception that is not caught (i.e. a number format exception since we parse a char). Debugging the application, I can see that the exception is thrown by the java thread library (which makes sense), but I cannot see where we should catch it. Note here that exception handling is already added when we click the save button (i.e. the check for an integer that is larger than 8 for the font size) but this bug happens before this step. Other possible fixes could however include:

  • Change the editable property to false. This would however mean that the user needs to press the up and down buttons multiple times.
  • Change the spinner to a simple text field. Then we can perform all the exception handling in the view/view model.

I think it will be great if we can fix it without changing the GUI, so if anyone has any comments or might want to work on this issue, just mention it here.

@Landi29
Copy link
Contributor Author

Landi29 commented Feb 3, 2021

I have found a fix where you bind an event handler directly to the TextField of the Spinner as returned by fontSize.getEditor(). The event handler is called when the Enter key is pressed. I know however that doing event handling directly on the controls can be a bit problematic, since it is not reusable code. A pull request will follow after I add some comments.

@Siedlerchr
Copy link
Member

Siedlerchr commented Feb 3, 2021

I think I have found a solution based on the TextFormatter the trick is to not return null:
Freel free to improve this

TextFormatter<Integer> formatter = new TextFormatter<>(
                                                              new IntegerStringConverter(),
                                                              9,
                                                              c -> {
                                                                  if (Pattern.matches("\\d*", c.getText())) {
                                                                      return c;
                                                                  }
                                                                  c.setText("0");
                                                                  return c;
                                                              });
       fontSize.getEditor().setTextFormatter(formatter);

When you enter a char it will return 0. Returning null results in an exception as well

@Landi29
Copy link
Contributor Author

Landi29 commented Feb 3, 2021

I will try it and see if it works.

@koobs
Copy link

koobs commented Feb 4, 2021

Could using ControlsFX (which jabref apparently uses?) Validation library help here?

@Siedlerchr
Copy link
Member

@koobs There is already a validation for that control, but that doesn't prevent the exception

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 good first issue An issue intended for project-newcomers. Varies in difficulty. preferences
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants