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.

Sign up for GitHub

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

Jump to bottom

Fix NPE and not on FX Thread in PreviewPrefs Tabs #4624

Merged
merged 6 commits into from
Feb 1, 2019
Merged

Fix NPE and not on FX Thread in PreviewPrefs Tabs #4624

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fada1d2
Fix NPE and not on FX Thread in PreviewPrefs Tabs
Siedlerchr Jan 31, 2019
59ae78c
fix checkstyle
Siedlerchr Jan 31, 2019
884d478
fix copying of entry preview
Siedlerchr Jan 31, 2019
c7e03d7
prevent divide by zero excpetion
Siedlerchr Jan 31, 2019
6649179
Pass task executor as variable
Siedlerchr Feb 1, 2019
0e5f84e
remove empty line
Siedlerchr Feb 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 19 additions & 17 deletions src/main/java/org/jabref/gui/PreviewPanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ public PreviewPanel(BasePanel panel, BibDatabaseContext databaseContext, KeyBind
this.keyBindingRepository = keyBindingRepository;

fileHandler = new NewDroppedFileHandler(dialogService, databaseContext, externalFileTypes,
Globals.prefs.getFilePreferences(),
Globals.prefs.getFilePreferences(),
Globals.prefs.getImportFormatPreferences(),
Globals.prefs.getUpdateFieldPreferences(),
Globals.getFileUpdateMonitor());
Globals.getFileUpdateMonitor());

// Set up scroll pane for preview pane
setFitToHeight(true);
Expand Down Expand Up @@ -231,19 +231,19 @@ private void updateLayout(PreviewPreferences previewPreferences, boolean init) {
if (CitationStyle.isCitationStyleFile(style)) {
layout = Optional.empty();
CitationStyle.createCitationStyleFromFile(style)
.ifPresent(cs -> {
citationStyle = cs;
if (!init) {
basePanel.get().output(Localization.lang("Preview style changed to: %0", citationStyle.getTitle()));
}
});
.ifPresent(cs -> {
citationStyle = cs;
if (!init) {
basePanel.get().output(Localization.lang("Preview style changed to: %0", citationStyle.getTitle()));
}
});
previewStyle = style;
} else {
previewStyle = defaultPreviewStyle;
updatePreviewLayout(previewPreferences.getPreviewStyle(), previewPreferences.getLayoutFormatterPreferences());
if (!init) {
basePanel.get().output(Localization.lang("Preview style changed to: %0", Localization.lang("Preview")));
}
}
} else {
previewStyle = defaultPreviewStyle;
updatePreviewLayout(previewPreferences.getPreviewStyle(), previewPreferences.getLayoutFormatterPreferences());
if (!init) {
basePanel.get().output(Localization.lang("Preview style changed to: %0", Localization.lang("Preview")));
}
}

Expand Down Expand Up @@ -301,7 +301,7 @@ public void update() {
.doLayout(entry, databaseContext.getDatabase())));
setPreviewLabel(sb.toString());
} else if (basePanel.isPresent() && bibEntry.isPresent()) {
if (citationStyle != null && !previewStyle.equals(defaultPreviewStyle)) {
if ((citationStyle != null) && !previewStyle.equals(defaultPreviewStyle)) {
basePanel.get().getCitationStyleCache().setCitationStyle(citationStyle);
}
Future<?> citationStyleWorker = BackgroundTask
Expand Down Expand Up @@ -368,11 +368,13 @@ public void close() {
private void copyPreviewToClipBoard() {
StringBuilder previewStringContent = new StringBuilder();
Document document = previewView.getEngine().getDocument();
NodeList nodeList = document.getElementsByTagName("*");

NodeList nodeList = document.getElementsByTagName("html");

//Nodelist does not implement iterable
for (int i = 0; i < nodeList.getLength(); i++) {
Element element = (Element) nodeList.item(i);
previewStringContent.append(element.getNodeValue()).append("\n");
previewStringContent.append(element.getTextContent());
}

ClipboardContent content = new ClipboardContent();
Expand Down
138 changes: 60 additions & 78 deletions src/main/java/org/jabref/gui/preferences/PreviewPrefsTab.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.ExecutionException;

import javax.swing.JPanel;
import javax.swing.SwingWorker;

import javafx.beans.binding.Bindings;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.concurrent.Task;
import javafx.scene.Node;
import javafx.scene.control.Button;
import javafx.scene.control.ButtonType;
Expand All @@ -28,7 +26,7 @@
import org.jabref.gui.DialogService;
import org.jabref.gui.PreviewPanel;
import org.jabref.gui.externalfiletype.ExternalFileTypes;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.util.TaskExecutor;
import org.jabref.logic.citationstyle.CitationStyle;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.TestEntry;
Expand All @@ -38,12 +36,10 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class PreviewPrefsTab extends JPanel implements PrefsTab {
public class PreviewPrefsTab implements PrefsTab {

private static final Logger LOGGER = LoggerFactory.getLogger(PreviewPrefsTab.class);

private SwingWorker<List<CitationStyle>, Void> discoverCitationStyleWorker;

private final ObservableList<Object> availableModel = FXCollections.observableArrayList();
private final ObservableList<Object> chosenModel = FXCollections.observableArrayList();

Expand All @@ -61,25 +57,24 @@ public class PreviewPrefsTab extends JPanel implements PrefsTab {
private final ScrollPane scrollPane = new ScrollPane(layout);
private final DialogService dialogService;
private final ExternalFileTypes externalFileTypes;
private final TaskExecutor taskExecutor;

private Task<List<CitationStyle>> citationDiscovery;

public PreviewPrefsTab(DialogService dialogService, ExternalFileTypes externalFileTypes) {
this.dialogService = dialogService;
this.externalFileTypes = externalFileTypes;
this.taskExecutor = Globals.TASK_EXECUTOR;
Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
setupLogic();
setupGui();
}

private void setupLogic() {
chosen.getSelectionModel().selectedItemProperty().addListener(event -> {
if (chosen.getSelectionModel().getSelectedIndices().isEmpty()) {
btnLeft.setDisable(true);
btnDown.setDisable(true);
btnUp.setDisable(true);
}
});

available.getSelectionModel().selectedItemProperty().addListener(
e -> btnRight.setDisable(available.getSelectionModel().getSelectedIndices().isEmpty()));
btnLeft.disableProperty().bind(Bindings.isEmpty(chosen.getSelectionModel().getSelectedItems()));
Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
btnDown.disableProperty().bind(Bindings.isEmpty(chosen.getSelectionModel().getSelectedItems()));
btnUp.disableProperty().bind(Bindings.isEmpty(chosen.getSelectionModel().getSelectedItems()));
btnRight.disableProperty().bind(Bindings.isEmpty(available.getSelectionModel().getSelectedItems()));

btnRight.setOnAction(event -> {
for (Object object : available.getSelectionModel().getSelectedItems()) {
Expand Down Expand Up @@ -110,9 +105,9 @@ private void setupLogic() {

btnDown.setOnAction(event -> {
List<Integer> newSelectedIndices = new ArrayList<>();
ObservableList selectedIndices = chosen.getSelectionModel().getSelectedIndices();
ObservableList<Integer> selectedIndices = chosen.getSelectionModel().getSelectedIndices();
for (int i = selectedIndices.size() - 1; i >= 0; i--) {
int oldIndex = (int)selectedIndices.get(i);
int oldIndex = selectedIndices.get(i);
boolean alreadyTaken = newSelectedIndices.contains(oldIndex + 1);
int newIndex = ((oldIndex < (chosenModel.size() - 1)) && !alreadyTaken) ? oldIndex + 1 : oldIndex;
chosenModel.add(newIndex, chosenModel.remove(oldIndex));
Expand All @@ -122,41 +117,34 @@ private void setupLogic() {
});

btnDefault.setOnAction(event -> layout.setText(Globals.prefs.getPreviewPreferences()
.getPreviewStyleDefault()
.replace("__NEWLINE__", "\n")));
.getPreviewStyleDefault()
.replace("__NEWLINE__", "\n")));

btnTest.setOnAction(event -> {
try {
DefaultTaskExecutor.runInJavaFXThread(() -> {
PreviewPanel testPane = new PreviewPanel(null, new BibDatabaseContext(), Globals.getKeyPrefs(), Globals.prefs.getPreviewPreferences(), dialogService, externalFileTypes);
if (chosen.getSelectionModel().getSelectedItems().isEmpty()) {
testPane.setFixedLayout(layout.getText());
testPane.setEntry(TestEntry.getTestEntry());
}
else {
int indexStyle = chosen.getSelectionModel().getSelectedIndex();
PreviewPreferences preferences = Globals.prefs.getPreviewPreferences();
preferences = new PreviewPreferences(preferences.getPreviewCycle(),indexStyle,preferences.getPreviewPanelDividerPosition(),preferences.isPreviewPanelEnabled(), preferences.getPreviewStyle(),preferences.getPreviewStyleDefault());

testPane = new PreviewPanel(JabRefGUI.getMainFrame().getCurrentBasePanel(), new BibDatabaseContext(), Globals.getKeyPrefs(), preferences, dialogService, externalFileTypes);
testPane.setEntry(TestEntry.getTestEntry());
testPane.updateLayout(preferences);
}

Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
DialogPane pane = new DialogPane();
pane.setContent(testPane);
PreviewPanel testPane = new PreviewPanel(null, new BibDatabaseContext(), Globals.getKeyPrefs(), Globals.prefs.getPreviewPreferences(), dialogService, externalFileTypes);
if (chosen.getSelectionModel().getSelectedItems().isEmpty()) {
testPane.setFixedLayout(layout.getText());
testPane.setEntry(TestEntry.getTestEntry());
} else {
int indexStyle = chosen.getSelectionModel().getSelectedIndex();
PreviewPreferences preferences = Globals.prefs.getPreviewPreferences();
preferences = new PreviewPreferences(preferences.getPreviewCycle(), indexStyle, preferences.getPreviewPanelDividerPosition(), preferences.isPreviewPanelEnabled(), preferences.getPreviewStyle(), preferences.getPreviewStyleDefault());

testPane = new PreviewPanel(JabRefGUI.getMainFrame().getCurrentBasePanel(), new BibDatabaseContext(), Globals.getKeyPrefs(), preferences, dialogService, externalFileTypes);
testPane.setEntry(TestEntry.getTestEntry());
testPane.updateLayout(preferences);
}

dialogService.showCustomDialogAndWait(Localization.lang("Preview"), pane, ButtonType.OK);
DialogPane pane = new DialogPane();
pane.setContent(testPane);

});
dialogService.showCustomDialogAndWait(Localization.lang("Preview"), pane, ButtonType.OK);

} catch (StringIndexOutOfBoundsException exception) {
LOGGER.warn("Parsing error.", exception);

DefaultTaskExecutor.runInJavaFXThread(()-> dialogService.showErrorDialogAndWait(Localization.lang("Parsing error"),
Localization.lang("Parsing error") + ": " + Localization.lang("illegal backslash expression"),
exception));

dialogService.showErrorDialogAndWait(Localization.lang("Parsing error"), Localization.lang("Parsing error") + ": " + Localization.lang("illegal backslash expression"), exception);
}
});
}
Expand All @@ -167,9 +155,9 @@ private void setupGui() {
btnLeft.setPrefSize(80, 20);
btnUp.setPrefSize(80, 20);
btnDown.setPrefSize(80, 20);
vBox.getChildren().addAll(new Label(""), new Label(""), new Label(""), new Label(""), new Label(""),
new Label(""), new Label(""), btnRight, btnLeft, new Label(""), new Label(""), new Label(""),
btnUp, btnDown);
vBox.getChildren().addAll(new Label(""), new Label(""), new Label(""), new Label(""), new Label(""),
new Label(""), new Label(""), btnRight, btnLeft, new Label(""), new Label(""), new Label(""),
btnUp, btnDown);
Label currentPreview = new Label(Localization.lang("Current Preview"));
currentPreview.getStyleClass().add("sectionHeader");
gridPane.add(currentPreview, 1, 1);
Expand All @@ -180,8 +168,10 @@ private void setupGui() {
gridPane.add(btnDefault, 3, 6);
layout.setPrefSize(600, 300);
gridPane.add(scrollPane, 1, 9);

Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public Node getBuilder() {
return gridPane;
}
Expand Down Expand Up @@ -212,46 +202,38 @@ public void setValues() {
availableModel.add(Localization.lang("Preview"));
}

btnLeft.setDisable(!chosen.getSelectionModel().getSelectedItems().isEmpty());
btnRight.setDisable(available.getSelectionModel().getSelectedIndices().isEmpty());
btnUp.setDisable(!chosen.getSelectionModel().getSelectedIndices().isEmpty());
btnDown.setDisable(!chosen.getSelectionModel().getSelectedIndices().isEmpty());

if (discoverCitationStyleWorker != null) {
discoverCitationStyleWorker.cancel(true);
if (citationDiscovery != null) {
citationDiscovery.cancel();
Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
}
citationDiscovery = new Task<List<CitationStyle>>() {

discoverCitationStyleWorker = new SwingWorker<List<CitationStyle>, Void>() {
@Override
protected List<CitationStyle> doInBackground() throws Exception {
protected List<CitationStyle> call() throws Exception {
return CitationStyle.discoverCitationStyles();
}

@Override
public void done() {
if (this.isCancelled()) {
return;
}
try {
get().stream()
.filter(style -> !previewPreferences.getPreviewCycle().contains(style.getFilePath()))
.sorted(Comparator.comparing(CitationStyle::getTitle))
.forEach(availableModel::add);

btnRight.setDisable(availableModel.isEmpty());
} catch (InterruptedException | ExecutionException e) {
LOGGER.error("something went wrong while adding the discovered CitationStyles to the list ");
}
}
};
discoverCitationStyleWorker.execute();

citationDiscovery.setOnSucceeded(value -> {
citationDiscovery.getValue().stream()
.filter(style -> !previewPreferences.getPreviewCycle().contains(style.getFilePath()))
.sorted(Comparator.comparing(CitationStyle::getTitle))
.forEach(availableModel::add);
});

citationDiscovery.setOnFailed(value -> LOGGER.error("something went wrong while adding the discovered CitationStyles to the list ", citationDiscovery.getException()));
Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
taskExecutor.execute(citationDiscovery);

layout.setText(Globals.prefs.getPreviewPreferences().getPreviewStyle().replace("__NEWLINE__", "\n"));
}

@Override
public void storeSettings() {
List<String> styles = new ArrayList<>();

if (chosenModel.isEmpty()) {
chosenModel.add(Localization.lang("Preview"));
}
for (Object obj : chosenModel) {
if (obj instanceof CitationStyle) {
styles.add(((CitationStyle) obj).getFilePath());
Expand All @@ -260,10 +242,10 @@ public void storeSettings() {
}
}
PreviewPreferences previewPreferences = Globals.prefs.getPreviewPreferences()
.getBuilder()
.withPreviewCycle(styles)
.withPreviewStyle(layout.getText().replace("\n", "__NEWLINE__"))
.build();
.getBuilder()
.withPreviewCycle(styles)
.withPreviewStyle(layout.getText().replace("\n", "__NEWLINE__"))
.build();
if (!chosen.getSelectionModel().isEmpty()) {
previewPreferences = previewPreferences.getBuilder().withPreviewCyclePosition(chosen.getSelectionModel().getSelectedIndex()).build();
}
Expand Down
16 changes: 10 additions & 6 deletions src/main/java/org/jabref/preferences/PreviewPreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
public class PreviewPreferences {

private final List<String> previewCycle;
private int previewCyclePosition;
private final int previewCyclePosition;
private final Number previewPanelDividerPosition;
private final boolean previewPanelEnabled;
private final String previewStyle;
Expand Down Expand Up @@ -60,14 +60,14 @@ public LayoutFormatterPreferences getLayoutFormatterPreferences() {
}

public static class Builder {

private List<String> previewCycle;
private int previeCyclePosition;
private Number previewPanelDividerPosition;
private boolean previewPanelEnabled;
private String previewStyle;
private final String previewStyleDefault;


public Builder(PreviewPreferences previewPreferences) {
this.previewCycle = previewPreferences.getPreviewCycle();
this.previeCyclePosition = previewPreferences.getPreviewCyclePosition();
Expand All @@ -83,11 +83,15 @@ public Builder withPreviewCycle(List<String> previewCycle) {
}

public Builder withPreviewCyclePosition(int position) {
previeCyclePosition = position;
while (previeCyclePosition < 0) {
previeCyclePosition += previewCycle.size();
if (previewCycle.isEmpty()) {
previeCyclePosition = 0;
} else {
previeCyclePosition = position;
while (previeCyclePosition < 0) {
previeCyclePosition += previewCycle.size();
}
previeCyclePosition %= previewCycle.size();
}
previeCyclePosition %= previewCycle.size();
return this;
}

Expand Down