fix loading and storing of XMP Field Exclusions

CHANGELOG.md

@@ -67,6 +67,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - We fixed an issue where users were vulnerable to XXE attacks during parsing [#4229](https://github.com/JabRef/jabref/issues/4229)
 - We fixed an issue where files added via the "Attach file" contextmenu of an entry were not made relative. [#4201](https://github.com/JabRef/jabref/issues/4201) and [#4241](https://github.com/JabRef/jabref/issues/4241)
 - We fixed an issue where author list parser can't generate bibtex for Chinese author. [#4169](https://github.com/JabRef/jabref/issues/4169)
+- We fixed an issue where the list of XMP Exclusion fields in the preferences was not be saved [#4072](https://github.com/JabRef/jabref/issues/4072)
 
 
 

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

@@ -1,29 +1,34 @@
 package org.jabref.gui.preferences;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 import java.util.stream.Collectors;
 
+import javafx.beans.property.ListProperty;
+import javafx.beans.property.SimpleListProperty;
 import javafx.beans.property.SimpleStringProperty;
+import javafx.beans.property.StringProperty;
 import javafx.collections.FXCollections;
-import javafx.collections.ObservableList;
 import javafx.scene.Node;
 import javafx.scene.control.Button;
 import javafx.scene.control.CheckBox;
+import javafx.scene.control.ComboBox;
 import javafx.scene.control.Label;
 import javafx.scene.control.ScrollPane;
 import javafx.scene.control.TableColumn;
+import javafx.scene.control.TableColumn.CellEditEvent;
 import javafx.scene.control.TableView;
-import javafx.scene.control.TextField;
-import javafx.scene.control.cell.PropertyValueFactory;
-import javafx.scene.control.cell.TextFieldTableCell;
 import javafx.scene.layout.BorderPane;
 import javafx.scene.layout.GridPane;
 import javafx.scene.layout.HBox;
 import javafx.scene.layout.Pane;
 
+import org.jabref.gui.icon.IconTheme;
+import org.jabref.gui.icon.IconTheme.JabRefIcons;
+import org.jabref.gui.util.ValueTableCellFactory;
 import org.jabref.logic.l10n.Localization;
+import org.jabref.model.entry.InternalBibtexFields;
+import org.jabref.model.strings.StringUtil;
 import org.jabref.preferences.JabRefPreferences;
 
 /**
@@ -32,34 +37,44 @@
  * Allows the user to enable and configure the XMP privacy filter.
  */
 class XmpPrefsTab extends Pane implements PrefsTab {
+
     private final JabRefPreferences prefs;
-    private boolean tableChanged;
     private final GridPane builder = new GridPane();
-    private final ObservableList<TableRow> data = FXCollections.observableArrayList();
+    private final ListProperty<XMPPrivacyFilter> fields = new SimpleListProperty<>(FXCollections.observableArrayList());
     private final CheckBox privacyFilterCheckBox = new CheckBox(
-            Localization.lang("Do not write the following fields to XMP Metadata:"));
-    private final List<TableRow> tableRows = new ArrayList<>(10);
+                                                                Localization.lang("Do not write the following fields to XMP Metadata:"));
+    private final TableView<XMPPrivacyFilter> tableView = new TableView<>();
 
     /**
      * Customization of external program paths.
      */
     public XmpPrefsTab(JabRefPreferences prefs) {
         this.prefs = Objects.requireNonNull(prefs);
-        TableView tableView = new TableView();
-        TableColumn<TableRow,String> column = new TableColumn<>(Localization.lang("Field to filter"));
-        column.setCellValueFactory(new PropertyValueFactory<>("name"));
-        column.setCellFactory(TextFieldTableCell.forTableColumn());
-        column.setOnEditCommit(
-                (TableColumn.CellEditEvent<TableRow, String> t) -> {
-                    t.getTableView().getItems().get(
-                            t.getTablePosition().getRow()).setName(t.getNewValue());
-                });
-        column.setPrefWidth(350);
-        tableView.setItems(data);
-        tableView.getColumns().add(column);
-        final TextField addName = new TextField();
-        addName.setPromptText("name");
-        addName.setPrefSize(200, 30);
+
+        tableView.itemsProperty().bindBidirectional(fields);
+        TableColumn<XMPPrivacyFilter, String> column = new TableColumn<>();
+        column.setCellValueFactory(cellData -> cellData.getValue().field());
+
+        TableColumn<XMPPrivacyFilter, String> deleteIconColumn = new TableColumn<>();
+        deleteIconColumn.setPrefWidth(60);
+        deleteIconColumn.setCellValueFactory(cellData -> cellData.getValue().field());
+        new ValueTableCellFactory<XMPPrivacyFilter, String>()
+        .withGraphic(item -> {
+            return IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode();
+        }).withOnMouseClickedEvent(item -> {
+            return evt -> delete();
+        }).install(deleteIconColumn);
+
+        column.setOnEditCommit((CellEditEvent<XMPPrivacyFilter, String> cell) -> {
+            cell.getRowValue().setField(cell.getNewValue());
+        });
+
+        tableView.getColumns().setAll(column, deleteIconColumn);
+        tableView.setColumnResizePolicy(TableView.CONSTRAINED_RESIZE_POLICY);
+
+        ComboBox<String> bibtexFields = new ComboBox<>(FXCollections.observableArrayList(InternalBibtexFields.getAllPublicAndInternalFieldNames()));
+        bibtexFields.setEditable(true);
+
         BorderPane tablePanel = new BorderPane();
         ScrollPane scrollPane = new ScrollPane();
         scrollPane.setMaxHeight(400);
@@ -68,34 +83,15 @@ public XmpPrefsTab(JabRefPreferences prefs) {
         tablePanel.setCenter(scrollPane);
 
         Button add = new Button("Add");
-        add.setPrefSize(80, 20);
-        add.setOnAction(e-> {
-            if (!addName.getText().isEmpty()) {
-                TableRow tableRow = new TableRow(addName.getText());
-                addName.clear();
-                data.add(tableRow);
-                tableRows.clear();
-                tableRows.addAll(data);
-                tableView.setItems(data);
-                tableChanged = true;
-                tableView.refresh();
-            }
-        });
-        Button delete = new Button("Delete");
-        delete.setPrefSize(80, 20);
-        delete.setOnAction(e-> {
-            if (tableView.getFocusModel() != null && tableView.getFocusModel().getFocusedIndex() != -1) {
-                tableChanged = true;
-                int row = tableView.getFocusModel().getFocusedIndex();
-                TableRow tableRow = data.get(row);
-                data.remove(tableRow);
-                tableRows.clear();
-                tableRows.addAll(data);
-                tableView.setItems(data);
-                tableView.refresh();
+        add.setGraphic(JabRefIcons.ADD.getGraphicNode());
+        add.setOnAction(e -> {
+            if (!StringUtil.isNullOrEmpty(bibtexFields.getSelectionModel().getSelectedItem())) {
+                XMPPrivacyFilter tableRow = new XMPPrivacyFilter(bibtexFields.getSelectionModel().getSelectedItem());
+                fields.add(tableRow);
             }
         });
-        HBox toolbar = new HBox(addName,add,delete);
+
+        HBox toolbar = new HBox(bibtexFields, add);
         tablePanel.setBottom(toolbar);
 
         // Build Prefs Tabs
@@ -105,37 +101,30 @@ public XmpPrefsTab(JabRefPreferences prefs) {
         builder.add(privacyFilterCheckBox, 1, 2);
         builder.add(tablePanel, 1, 3);
 
+        tableView.disableProperty().bind(privacyFilterCheckBox.selectedProperty().not());
+        add.disableProperty().bind(privacyFilterCheckBox.selectedProperty().not());
     }
 
-    public Node getBuilder() {
-        return builder;
-    }
-
-    public static class TableRow {
-        private SimpleStringProperty name;
-
-        TableRow(String name) {
-            this.name = new SimpleStringProperty(name);
-        }
-
-        public void setName(String name) {
-            this.name.set(name);
+    private void delete() {
+        if (tableView.getSelectionModel().getSelectedItem() != null) {
+            XMPPrivacyFilter tableRow = tableView.getSelectionModel().getSelectedItem();
+            fields.remove(tableRow);
         }
+    }
 
-        public String getName() {
-            return name.get();
-        }
+    @Override
+    public Node getBuilder() {
+        return builder;
     }
 
     /**
      * Load settings from the preferences and initialize the table.
      */
     @Override
     public void setValues() {
-        tableRows.clear();
-        tableRows.addAll(data);
-        privacyFilterCheckBox.setSelected(JabRefPreferences.getInstance().getBoolean(
-                JabRefPreferences.USE_XMP_PRIVACY_FILTER));
+        List<XMPPrivacyFilter> xmpExclusions = prefs.getStringList(JabRefPreferences.XMP_PRIVACY_FILTERS).stream().map(XMPPrivacyFilter::new).collect(Collectors.toList());
+        fields.setAll(xmpExclusions);
+        privacyFilterCheckBox.setSelected(JabRefPreferences.getInstance().getBoolean(JabRefPreferences.USE_XMP_PRIVACY_FILTER));
     }
 
     /**
@@ -145,24 +134,11 @@ public void setValues() {
      */
     @Override
     public void storeSettings() {
-        // Now we need to make sense of the contents the user has made to the
-        // table setup table. This needs to be done either if changes were made, or
-        // if the checkbox is checked and no field values have been stored previously:
-        if (tableChanged ||
-                (privacyFilterCheckBox.isSelected() && !prefs.hasKey(JabRefPreferences.XMP_PRIVACY_FILTERS))) {
-
-            // First we remove all rows with empty names.
-            for (int i = tableRows.size() - 1; i >= 0; i--) {
-                if ((tableRows.get(i) == null) || tableRows.get(i).toString().isEmpty()) {
-                    tableRows.remove(i);
-                }
-            }
-            // Finally, we store the new preferences.
-            JabRefPreferences.getInstance().putStringList(JabRefPreferences.XMP_PRIVACY_FILTERS,
-                    tableRows.stream().map(Object::toString).collect(Collectors.toList()));
-        }
 
-        JabRefPreferences.getInstance().putBoolean(JabRefPreferences.USE_XMP_PRIVACY_FILTER, privacyFilterCheckBox.isSelected());
+        fields.stream().filter(s -> StringUtil.isNullOrEmpty(s.getField())).forEach(fields::remove);
+        prefs.putStringList(JabRefPreferences.XMP_PRIVACY_FILTERS,
+                            fields.stream().map(XMPPrivacyFilter::getField).collect(Collectors.toList()));
+        prefs.putBoolean(JabRefPreferences.USE_XMP_PRIVACY_FILTER, privacyFilterCheckBox.isSelected());
     }
 
     @Override
@@ -174,4 +150,30 @@ public boolean validateSettings() {
     public String getTabName() {
         return Localization.lang("XMP-metadata");
     }
+
+    private class XMPPrivacyFilter {
+
+        private final SimpleStringProperty field;
+
+        XMPPrivacyFilter(String field) {
+            this.field = new SimpleStringProperty(field);
+        }
+
+        public void setField(String field) {
+            this.field.set(field);
+        }
+
+        public String getField() {
+            return field.get();
+        }
+
+        public StringProperty field() {
+            return field;
+        }
+
+        @Override
+        public String toString() {
+            return field.getValue();
+        }
+    }
 }

src/main/java/org/jabref/gui/util/ValueTableCellFactory.java

@@ -122,7 +122,7 @@ protected void updateItem(T item, boolean empty) {
                             toOnMouseClickedEvent.apply(rowItem, item).handle(event);
                         }
 
-                        if (menuFactory != null && !event.isConsumed()) {
+                        if ((menuFactory != null) && !event.isConsumed()) {
                             if (event.getButton() == MouseButton.PRIMARY) {
                                 ContextMenu menu = menuFactory.apply(rowItem, item);
                                 if (menu != null) {

src/main/resources/l10n/JabRef_en.properties

@@ -420,8 +420,6 @@ field=field
 Field\ name=Field name
 Field\ names\ are\ not\ allowed\ to\ contain\ white\ space\ or\ the\ following\ characters=Field names are not allowed to contain white space or the following characters
 
-Field\ to\ filter=Field to filter
-
 Field\ to\ group\ by=Field to group by
 
 File=File