Fix for multiple error messages in messed up source editor (#5823)

* Fix for unknown gray shade in dark theme

* Fix for missing import in module system

* Fixed multiple error dialogs with the same message

* Refactored FunctionBasedValidators to common interface and harmonised their logic

* Fixed storing entry before changing it and validation

* Fixes sync between fields and sourceTab

* Fixes test

src/main/java/module-info.java

@@ -68,4 +68,5 @@
     requires org.jsoup;
     requires commons.csv;
     requires io.github.javadiffutils;
+    requires flowless;
 }

src/main/java/org/jabref/gui/Dark.css

@@ -60,7 +60,7 @@
 }
 
 .table-view .groupColumnBackground {
-    -fx-stroke: -jr-gray-4;
+    -fx-stroke: -jr-gray-3;
 }
 
 .code-area .lineno {

src/main/java/org/jabref/gui/entryeditor/SourceTab.java

@@ -11,9 +11,9 @@
 
 import javax.swing.undo.UndoManager;
 
+import javafx.beans.InvalidationListener;
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleObjectProperty;
-import javafx.collections.ListChangeListener;
 import javafx.geometry.Point2D;
 import javafx.scene.control.ContextMenu;
 import javafx.scene.control.Tooltip;
@@ -31,7 +31,6 @@
 import org.jabref.gui.undo.NamedCompound;
 import org.jabref.gui.undo.UndoableChangeType;
 import org.jabref.gui.undo.UndoableFieldChange;
-import org.jabref.gui.util.BindingsHelper;
 import org.jabref.gui.util.DefaultTaskExecutor;
 import org.jabref.logic.bibtex.BibEntryWriter;
 import org.jabref.logic.bibtex.InvalidFieldValueException;
@@ -51,7 +50,7 @@
 
 import de.saxsys.mvvmfx.utils.validation.ObservableRuleBasedValidator;
 import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
-import org.controlsfx.control.NotificationPane;
+import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
 import org.fxmisc.flowless.VirtualizedScrollPane;
 import org.fxmisc.richtext.CodeArea;
 import org.slf4j.Logger;
@@ -64,15 +63,17 @@ public class SourceTab extends EntryEditorTab {
     private final BibDatabaseMode mode;
     private final UndoManager undoManager;
     private final ObjectProperty<ValidationMessage> sourceIsValid = new SimpleObjectProperty<>();
-    @SuppressWarnings("unchecked") private final ObservableRuleBasedValidator sourceValidator = new ObservableRuleBasedValidator(sourceIsValid);
+    private final ObservableRuleBasedValidator sourceValidator = new ObservableRuleBasedValidator();
     private final ImportFormatPreferences importFormatPreferences;
     private final FileUpdateMonitor fileMonitor;
     private final DialogService dialogService;
     private final StateManager stateManager;
     private Optional<Pattern> searchHighlightPattern = Optional.empty();
-    private CodeArea codeArea;
+    private final CodeArea codeArea;
     private KeyBindingRepository keyBindingRepository;
 
+    private BibEntry previousEntry;
+
     private class EditAction extends SimpleCommand {
 
         private final StandardActions command;
@@ -81,7 +82,6 @@ private class EditAction extends SimpleCommand {
 
         @Override
         public void execute() {
-            if (codeArea != null) {
                 switch (command) {
                     case COPY:
                         codeArea.copy();
@@ -97,7 +97,6 @@ public void execute() {
                         break;
                 }
                 codeArea.requestFocus();
-            }
         }
     }
 
@@ -113,6 +112,11 @@ public SourceTab(BibDatabaseContext bibDatabaseContext, CountingUndoManager undo
         this.dialogService = dialogService;
         this.stateManager = stateManager;
         this.keyBindingRepository = keyBindingRepository;
+        this.codeArea = new CodeArea();
+
+        setupSourceEditor();
+        VirtualizedScrollPane<CodeArea> scrollableCodeArea = new VirtualizedScrollPane<>(codeArea);
+        this.setContent(scrollableCodeArea);
 
         stateManager.activeSearchQueryProperty().addListener((observable, oldValue, newValue) -> {
             searchHighlightPattern = newValue.flatMap(SearchQuery::getPatternForWords);
@@ -144,7 +148,7 @@ private static String getSourceString(BibEntry entry, BibDatabaseMode type, Late
     /* Work around for different input methods.
      * https://github.com/FXMisc/RichTextFX/issues/146
      */
-    private class InputMethodRequestsObject implements InputMethodRequests {
+    private static class InputMethodRequestsObject implements InputMethodRequests {
 
         @Override
         public String getSelectedText() {
@@ -167,8 +171,7 @@ public Point2D getTextLocation(int offset) {
         }
     }
 
-    private CodeArea createSourceEditor() {
-        CodeArea codeArea = new CodeArea();
+    private void setupSourceEditor() {
         codeArea.setWrapText(true);
         codeArea.setInputMethodRequests(new InputMethodRequestsObject());
         codeArea.setOnInputMethodTextChanged(event -> {
@@ -191,54 +194,53 @@ private CodeArea createSourceEditor() {
         contextMenu.getStyleClass().add("context-menu");
         codeArea.setContextMenu(contextMenu);
 
-        return codeArea;
+        sourceValidator.addRule(sourceIsValid);
+
+        sourceValidator.getValidationStatus().getMessages().addListener((InvalidationListener) c -> {
+            ValidationStatus sourceValidationStatus = sourceValidator.getValidationStatus();
+            if (!sourceValidationStatus.isValid()) {
+                sourceValidationStatus.getHighestMessage().ifPresent(message ->
+                    dialogService.showErrorDialogAndWait(message.getMessage()));
+            }
+        });
+
+        codeArea.focusedProperty().addListener((obs, oldValue, onFocus) -> {
+            if (!onFocus && currentEntry != null) {
+                storeSource(currentEntry, codeArea.textProperty().getValue());
+            }
+        });
     }
 
     @Override
     public boolean shouldShow(BibEntry entry) {
         return true;
     }
 
-    @Override
-    protected void bindToEntry(BibEntry entry) {
-        CodeArea codeArea = createSourceEditor();
-        VirtualizedScrollPane<CodeArea> node = new VirtualizedScrollPane<>(codeArea);
-        NotificationPane notificationPane = new NotificationPane(node);
-        notificationPane.setShowFromTop(false);
-        sourceValidator.getValidationStatus().getMessages().addListener((ListChangeListener<ValidationMessage>) c -> {
-            if (sourceValidator.getValidationStatus().isValid()) {
-                notificationPane.hide();
-            } else {
-                sourceValidator.getValidationStatus().getHighestMessage().ifPresent(validationMessage -> {
-                    notificationPane.show(validationMessage.getMessage());//this seems not working
-                    dialogService.showErrorDialogAndWait(validationMessage.getMessage());
-                });
+    private void updateCodeArea() {
+        DefaultTaskExecutor.runAndWaitInJavaFXThread(() -> {
+            codeArea.clear();
+            try {
+                codeArea.appendText(getSourceString(currentEntry, mode, fieldFormatterPreferences));
+                highlightSearchPattern();
+            } catch (IOException ex) {
+                codeArea.setEditable(false);
+                codeArea.appendText(ex.getMessage() + "\n\n" +
+                        Localization.lang("Correct the entry, and reopen editor to display/edit source."));
+                LOGGER.debug("Incorrect entry", ex);
             }
         });
-        this.setContent(codeArea);
-        this.codeArea = codeArea;
-
-        // Store source for on focus out event in the source code (within its text area)
-        // and update source code for every change of entry field values
-        BindingsHelper.bindContentBidirectional(entry.getFieldsObservable(), codeArea.focusedProperty(), onFocus -> {
-            if (!onFocus) {
-                storeSource(entry, codeArea.textProperty().getValue());
-            }
-        }, fields -> {
-            DefaultTaskExecutor.runAndWaitInJavaFXThread(() -> {
-                codeArea.clear();
-                try {
-                    codeArea.appendText(getSourceString(entry, mode, fieldFormatterPreferences));
-                    highlightSearchPattern();
-                } catch (IOException ex) {
-                    codeArea.setEditable(false);
-                    codeArea.appendText(ex.getMessage() + "\n\n" +
-                                        Localization.lang("Correct the entry, and reopen editor to display/edit source."));
-                    LOGGER.debug("Incorrect entry", ex);
-                }
-            });
-        });
+    }
+
+    @Override
+    protected void bindToEntry(BibEntry entry) {
+        if (previousEntry != null) {
+            storeSource(previousEntry, codeArea.textProperty().getValue());
+        }
+        this.previousEntry = entry;
+
+        updateCodeArea();
 
+        entry.getFieldsObservable().addListener((InvalidationListener) listener -> updateCodeArea());
     }
 
     private void storeSource(BibEntry outOfFocusEntry, String text) {

src/main/java/org/jabref/gui/preferences/AdvancedTabViewModel.java

@@ -25,6 +25,7 @@
 import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
 import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
 import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
+import de.saxsys.mvvmfx.utils.validation.Validator;
 
 public class AdvancedTabViewModel implements PreferenceTabViewModel {
     private final BooleanProperty remoteServerProperty = new SimpleBooleanProperty();
@@ -39,11 +40,11 @@ public class AdvancedTabViewModel implements PreferenceTabViewModel {
     private final StringProperty proxyUsernameProperty = new SimpleStringProperty("");
     private final StringProperty proxyPasswordProperty = new SimpleStringProperty("");
 
-    private FunctionBasedValidator remotePortValidator;
-    private FunctionBasedValidator proxyHostnameValidator;
-    private FunctionBasedValidator proxyPortValidator;
-    private FunctionBasedValidator proxyUsernameValidator;
-    private FunctionBasedValidator proxyPasswordValidator;
+    private Validator remotePortValidator;
+    private Validator proxyHostnameValidator;
+    private Validator proxyPortValidator;
+    private Validator proxyUsernameValidator;
+    private Validator proxyPasswordValidator;
 
     private final DialogService dialogService;
     private final JabRefPreferences preferences;

src/main/java/org/jabref/gui/preferences/AppearanceTabViewModel.java

@@ -16,6 +16,7 @@
 import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
 import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
 import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
+import de.saxsys.mvvmfx.utils.validation.Validator;
 
 public class AppearanceTabViewModel implements PreferenceTabViewModel {
 
@@ -27,7 +28,7 @@ public class AppearanceTabViewModel implements PreferenceTabViewModel {
     private final DialogService dialogService;
     private final JabRefPreferences preferences;
 
-    private FunctionBasedValidator fontSizeValidator;
+    private Validator fontSizeValidator;
 
     private List<String> restartWarnings = new ArrayList<>();
 
@@ -93,12 +94,10 @@ public void storeSettings() {
 
     @Override
     public boolean validateSettings() {
-        if (fontOverrideProperty.getValue()) {
-            if (!fontSizeValidator.getValidationStatus().isValid()) {
-                fontSizeValidator.getValidationStatus().getHighestMessage().ifPresent(message ->
-                        dialogService.showErrorDialogAndWait(message.getMessage()));
-                return false;
-            }
+        if (fontOverrideProperty.getValue() && !fontSizeValidator.getValidationStatus().isValid()) {
+            fontSizeValidator.getValidationStatus().getHighestMessage().ifPresent(message ->
+                    dialogService.showErrorDialogAndWait(message.getMessage()));
+            return false;
         }
         return true;
     }

src/main/java/org/jabref/gui/preferences/FileTabViewModel.java

@@ -27,6 +27,7 @@
 import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
 import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
 import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
+import de.saxsys.mvvmfx.utils.validation.Validator;
 
 public class FileTabViewModel implements PreferenceTabViewModel {
 
@@ -51,7 +52,7 @@ public class FileTabViewModel implements PreferenceTabViewModel {
 
     private final BooleanProperty autosaveLocalLibraries = new SimpleBooleanProperty();
 
-    private final FunctionBasedValidator mainFileDirValidator;
+    private final Validator mainFileDirValidator;
 
     private final DialogService dialogService;
     private final JabRefPreferences preferences;
@@ -136,9 +137,10 @@ ValidationStatus mainFileDirValidationStatus() {
 
     @Override
     public boolean validateSettings() {
-        ValidationStatus status = mainFileDirValidationStatus();
-        if (!status.isValid()) {
-            dialogService.showErrorDialogAndWait(status.getHighestMessage().get().getMessage());
+        ValidationStatus validationStatus = mainFileDirValidationStatus();
+        if (!validationStatus.isValid()) {
+            validationStatus.getHighestMessage().ifPresent(message ->
+                    dialogService.showErrorDialogAndWait(message.getMessage()));
             return false;
         }
         return true;

src/main/java/org/jabref/gui/preferences/GeneralTabViewModel.java

@@ -25,6 +25,7 @@
 import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
 import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
 import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
+import de.saxsys.mvvmfx.utils.validation.Validator;
 
 public class GeneralTabViewModel implements PreferenceTabViewModel {
     private final ListProperty<Language> languagesListProperty = new SimpleListProperty<>();
@@ -50,7 +51,7 @@ public class GeneralTabViewModel implements PreferenceTabViewModel {
     private final StringProperty markTimeStampFieldNameProperty = new SimpleStringProperty("");
     private final BooleanProperty updateTimeStampProperty = new SimpleBooleanProperty();
 
-    private FunctionBasedValidator markTimeStampFormatValidator;
+    private Validator markTimeStampFormatValidator;
 
     private final DialogService dialogService;
     private final JabRefPreferences preferences;

src/main/java/org/jabref/gui/preferences/PreviewTabViewModel.java

@@ -43,6 +43,7 @@
 import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
 import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
 import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
+import de.saxsys.mvvmfx.utils.validation.Validator;
 import org.fxmisc.richtext.model.StyleSpans;
 import org.fxmisc.richtext.model.StyleSpansBuilder;
 import org.slf4j.Logger;
@@ -65,7 +66,7 @@ public class PreviewTabViewModel implements PreferenceTabViewModel {
     private final PreviewPreferences previewPreferences;
     private final TaskExecutor taskExecutor;
     private final CustomLocalDragboard localDragboard = GUIGlobals.localDragboard;
-    private FunctionBasedValidator chosenListValidator;
+    private Validator chosenListValidator;
     private ListProperty<PreviewLayout> dragSourceList = null;
 
     public PreviewTabViewModel(DialogService dialogService, JabRefPreferences preferences, TaskExecutor taskExecutor) {

src/main/java/org/jabref/gui/preferences/TableColumnsTabViewModel.java

@@ -32,6 +32,7 @@
 import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
 import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
 import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
+import de.saxsys.mvvmfx.utils.validation.Validator;
 
 public class TableColumnsTabViewModel implements PreferenceTabViewModel {
 
@@ -60,7 +61,7 @@ public MainTableColumnModel fromString(String string) {
     private final BooleanProperty specialFieldsSerializeProperty = new SimpleBooleanProperty();
     private final BooleanProperty extraFileColumnsEnabledProperty = new SimpleBooleanProperty();
 
-    private FunctionBasedValidator<?> columnsNotEmptyValidator;
+    private Validator columnsNotEmptyValidator;
 
     private List<String> restartWarnings = new ArrayList<>();
 
@@ -239,9 +240,10 @@ ValidationStatus columnsListValidationStatus() {
 
     @Override
     public boolean validateSettings() {
-        ValidationStatus status = columnsListValidationStatus();
-        if (!status.isValid() && status.getHighestMessage().isPresent()) {
-            dialogService.showErrorDialogAndWait(status.getHighestMessage().get().getMessage());
+        ValidationStatus validationStatus = columnsListValidationStatus();
+        if (!validationStatus.isValid()) {
+            validationStatus.getHighestMessage().ifPresent(message ->
+                    dialogService.showErrorDialogAndWait(message.getMessage()));
             return false;
         }
         return true;

src/main/java/org/jabref/gui/preferences/XmpPrivacyTabViewModel.java

@@ -21,6 +21,7 @@
 import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
 import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
 import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
+import de.saxsys.mvvmfx.utils.validation.Validator;
 
 public class XmpPrivacyTabViewModel implements PreferenceTabViewModel {
 
@@ -32,7 +33,7 @@ public class XmpPrivacyTabViewModel implements PreferenceTabViewModel {
     private final DialogService dialogService;
     private final JabRefPreferences preferences;
 
-    private FunctionBasedValidator xmpFilterListValidator;
+    private Validator xmpFilterListValidator;
 
     XmpPrivacyTabViewModel(DialogService dialogService, JabRefPreferences preferences) {
         this.dialogService = dialogService;

src/main/java/org/jabref/gui/texparser/ParseTexDialogViewModel.java

@@ -37,6 +37,7 @@
 import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
 import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
 import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
+import de.saxsys.mvvmfx.utils.validation.Validator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -50,7 +51,7 @@ public class ParseTexDialogViewModel extends AbstractViewModel {
     private final PreferencesService preferencesService;
     private final FileUpdateMonitor fileMonitor;
     private final StringProperty texDirectory;
-    private final FunctionBasedValidator<String> texDirectoryValidator;
+    private final Validator texDirectoryValidator;
     private final ObjectProperty<FileNodeViewModel> root;
     private final ObservableList<TreeItem<FileNodeViewModel>> checkedFileList;
     private final BooleanProperty noFilesFound;

src/test/java/org/jabref/gui/entryeditor/SourceTabTest.java

@@ -78,7 +78,7 @@ void switchingFromSourceTabDoesNotThrowException(FxRobot robot) throws Exception
 
         // Update source editor
         robot.interact(() -> pane.getSelectionModel().select(2));
-        robot.interact(() -> sourceTab.bindToEntry(entry));
+        robot.interact(() -> sourceTab.notifyAboutFocus(entry));
         robot.clickOn(1200, 500);
         robot.interrupt(100);