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

Fix for issue 8747: date field change causes an uncaught exception #9314

Closed
wants to merge 9 commits into from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where hitting enter on the search field within the preferences dialog closed the dialog. [koppor#630](https://github.com/koppor/jabref/issues/630)
- We fixed a typo within a connection error message. [koppor#625](https://github.com/koppor/jabref/issues/625)
- We fixed an issue where the 'close dialog' key binding was not closing the Preferences dialog. [#8888](https://github.com/jabref/jabref/issues/8888)
- We fixed an issue where editing entry's "date" field in library mode "biblatex" causes an uncaught exception. [#8747][https://github.com/JabRef/jabref/issues/8747]

### Removed

Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/jabref/gui/fieldeditors/DateEditor.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.jabref.logic.integrity.FieldCheckers;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.Field;
import org.jabref.preferences.PreferencesService;

import com.airhacks.afterburner.views.ViewLoader;

Expand All @@ -19,7 +20,7 @@ public class DateEditor extends HBox implements FieldEditorFX {
@FXML private DateEditorViewModel viewModel;
@FXML private TemporalAccessorPicker datePicker;

public DateEditor(Field field, DateTimeFormatter dateFormatter, SuggestionProvider<?> suggestionProvider, FieldCheckers fieldCheckers) {
public DateEditor(Field field, DateTimeFormatter dateFormatter, SuggestionProvider<?> suggestionProvider, FieldCheckers fieldCheckers, PreferencesService preferences) {
this.viewModel = new DateEditorViewModel(field, suggestionProvider, dateFormatter, fieldCheckers);

ViewLoader.view(this)
Expand All @@ -28,6 +29,8 @@ public DateEditor(Field field, DateTimeFormatter dateFormatter, SuggestionProvid

datePicker.setStringConverter(viewModel.getDateToStringConverter());
datePicker.getEditor().textProperty().bindBidirectional(viewModel.textProperty());

new EditorValidator(preferences).configureValidation(viewModel.getFieldValidator().getValidationStatus(), datePicker.getEditor());
}

public DateEditorViewModel getViewModel() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public TemporalAccessor fromString(String string) {
if (StringUtil.isNotBlank(string)) {
try {
return dateFormatter.parse(string);
} catch (DateTimeParseException exception) {
} catch (Exception exception) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change this to a generic exception? Exception should be always the most specfic one

// We accept all kinds of dates (not just in the format specified)
return Date.parse(string).map(Date::toTemporalAccessor).orElse(null);
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/fieldeditors/FieldEditors.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ public static FieldEditorFX getForField(final Field field,
boolean isMultiLine = FieldFactory.isMultiLineField(field, preferences.getFieldContentParserPreferences().getNonWrappableFields());

if (preferences.getTimestampPreferences().getTimestampField().equals(field)) {
return new DateEditor(field, DateTimeFormatter.ofPattern(preferences.getTimestampPreferences().getTimestampFormat()), suggestionProvider, fieldCheckers);
return new DateEditor(field, DateTimeFormatter.ofPattern(preferences.getTimestampPreferences().getTimestampFormat()), suggestionProvider, fieldCheckers, preferences);
} else if (fieldProperties.contains(FieldProperty.DATE)) {
return new DateEditor(field, DateTimeFormatter.ofPattern("[uuuu][-MM][-dd]"), suggestionProvider, fieldCheckers);
return new DateEditor(field, DateTimeFormatter.ofPattern("[uuuu][-MM][-dd]"), suggestionProvider, fieldCheckers, preferences);
} else if (fieldProperties.contains(FieldProperty.EXTERNAL)) {
return new UrlEditor(field, dialogService, suggestionProvider, fieldCheckers, preferences);
} else if (fieldProperties.contains(FieldProperty.JOURNAL_NAME)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.time.Year;
import java.time.YearMonth;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAccessor;
import java.time.temporal.TemporalQueries;
import java.util.Objects;
Expand All @@ -21,6 +22,8 @@
import org.jabref.gui.fieldeditors.TextInputControlBehavior;
import org.jabref.gui.fieldeditors.contextmenu.EditorContextAction;
import org.jabref.gui.util.BindingsHelper;
import org.jabref.model.entry.Date;
import org.jabref.model.strings.StringUtil;

/**
* A date picker with configurable datetime format where both date and time can be changed via the text field and the
Expand Down Expand Up @@ -75,6 +78,11 @@ private static LocalDate getDate(TemporalAccessor temporalAccessor) {
}

private static LocalDate getLocalDate(TemporalAccessor dateTime) {
// Return null when dateTime is null pointer
if (dateTime == null) {
return null;
}

// Try to get as much information from the temporal accessor
LocalDate date = dateTime.query(TemporalQueries.localDate());
if (date != null) {
Expand All @@ -101,7 +109,16 @@ public String toString(TemporalAccessor value) {

@Override
public TemporalAccessor fromString(String value) {
return LocalDateTime.parse(value, defaultFormatter);
if (StringUtil.isNotBlank(value)) {
try {
return defaultFormatter.parse(value);
} catch (
DateTimeParseException exception) {
return Date.parse(value).map(Date::toTemporalAccessor).orElse(null);
}
} else {
return null;
}
}
};
return Objects.requireNonNullElseGet(stringConverterProperty().get(), () -> newConverter);
Expand All @@ -127,7 +144,9 @@ private class InternalConverter extends StringConverter<LocalDate> {
@Override
public String toString(LocalDate object) {
TemporalAccessor value = getTemporalAccessorValue();
return (value != null) ? getStringConverter().toString(value) : "";

// Keeps the original text when it is an invalid date
return (value != null) ? getStringConverter().toString(value) : getEditor().getText();
}

@Override
Expand All @@ -139,6 +158,7 @@ public LocalDate fromString(String value) {

TemporalAccessor dateTime = getStringConverter().fromString(value);
temporalAccessorValue.set(dateTime);

return getLocalDate(dateTime);
}
}
Expand Down