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 NPE and not on FX Thread in PreviewPrefs Tabs #4624

Merged
merged 6 commits into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 13 additions & 14 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -923,24 +923,23 @@ private MenuBar createMenu() {
);

options.getItems().addAll(
factory.createMenuItem(StandardActions.SHOW_PREFS, new ShowPreferencesAction(this)),
factory.createMenuItem(StandardActions.SHOW_PREFS, new ShowPreferencesAction(this, Globals.TASK_EXECUTOR)),

new SeparatorMenuItem(),
new SeparatorMenuItem(),

factory.createMenuItem(StandardActions.SETUP_GENERAL_FIELDS, new SetupGeneralFieldsAction()),
factory.createMenuItem(StandardActions.MANAGE_CUSTOM_IMPORTS, new ManageCustomImportsAction()),
factory.createMenuItem(StandardActions.MANAGE_CUSTOM_EXPORTS, new ManageCustomExportsAction()),
factory.createMenuItem(StandardActions.MANAGE_EXTERNAL_FILETYPES, new EditExternalFileTypesAction()),
factory.createMenuItem(StandardActions.MANAGE_JOURNALS, new ManageJournalsAction()),
factory.createMenuItem(StandardActions.CUSTOMIZE_KEYBINDING, new CustomizeKeyBindingAction()),
factory.createMenuItem(StandardActions.MANAGE_PROTECTED_TERMS, new ManageProtectedTermsAction(this, Globals.protectedTermsLoader)),
factory.createMenuItem(StandardActions.SETUP_GENERAL_FIELDS, new SetupGeneralFieldsAction()),
factory.createMenuItem(StandardActions.MANAGE_CUSTOM_IMPORTS, new ManageCustomImportsAction()),
factory.createMenuItem(StandardActions.MANAGE_CUSTOM_EXPORTS, new ManageCustomExportsAction()),
factory.createMenuItem(StandardActions.MANAGE_EXTERNAL_FILETYPES, new EditExternalFileTypesAction()),
factory.createMenuItem(StandardActions.MANAGE_JOURNALS, new ManageJournalsAction()),
factory.createMenuItem(StandardActions.CUSTOMIZE_KEYBINDING, new CustomizeKeyBindingAction()),
factory.createMenuItem(StandardActions.MANAGE_PROTECTED_TERMS, new ManageProtectedTermsAction(this, Globals.protectedTermsLoader)),

new SeparatorMenuItem(),
new SeparatorMenuItem(),

factory.createMenuItem(StandardActions.MANAGE_CONTENT_SELECTORS, new OldDatabaseCommandWrapper(Actions.MANAGE_SELECTORS, this, Globals.stateManager)),
factory.createMenuItem(StandardActions.CUSTOMIZE_ENTRY_TYPES, new CustomizeEntryAction(this)),
factory.createMenuItem(StandardActions.MANAGE_CITE_KEY_PATTERNS, new BibtexKeyPatternAction(this))
);
factory.createMenuItem(StandardActions.MANAGE_CONTENT_SELECTORS, new OldDatabaseCommandWrapper(Actions.MANAGE_SELECTORS, this, Globals.stateManager)),
factory.createMenuItem(StandardActions.CUSTOMIZE_ENTRY_TYPES, new CustomizeEntryAction(this)),
factory.createMenuItem(StandardActions.MANAGE_CITE_KEY_PATTERNS, new BibtexKeyPatternAction(this)));

help.getItems().addAll(
factory.createMenuItem(StandardActions.HELP, HelpAction.getMainHelpPageCommand()),
Expand Down
36 changes: 19 additions & 17 deletions src/main/java/org/jabref/gui/PreviewPanel.java
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,21 @@

import org.jabref.gui.JabRefFrame;
import org.jabref.gui.preferences.PreferencesDialog;
import org.jabref.gui.util.TaskExecutor;

public class ShowPreferencesAction extends SimpleCommand {

private final JabRefFrame jabRefFrame;
public ShowPreferencesAction(JabRefFrame jabRefFrame) {
private final TaskExecutor taskExecutor;

public ShowPreferencesAction(JabRefFrame jabRefFrame, TaskExecutor taskExecutor) {
this.jabRefFrame = jabRefFrame;
this.taskExecutor = taskExecutor;
}

@Override
public void execute() {
PreferencesDialog preferencesDialog = new PreferencesDialog(jabRefFrame);
PreferencesDialog preferencesDialog = new PreferencesDialog(jabRefFrame, taskExecutor);
preferencesDialog.showAndWait();
}
}
53 changes: 26 additions & 27 deletions src/main/java/org/jabref/gui/preferences/PreferencesDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.jabref.gui.util.BaseDialog;
import org.jabref.gui.util.ControlHelper;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.gui.util.TaskExecutor;
import org.jabref.gui.util.ViewModelListCellFactory;
import org.jabref.logic.exporter.ExporterFactory;
import org.jabref.logic.exporter.SavePreferences;
Expand Down Expand Up @@ -57,7 +58,7 @@ public class PreferencesDialog extends BaseDialog<Void> {
private final JabRefPreferences prefs;
private final ObservableList<PrefsTab> preferenceTabs;

public PreferencesDialog(JabRefFrame parent) {
public PreferencesDialog(JabRefFrame parent, TaskExecutor taskExecutor) {
setTitle(Localization.lang("JabRef preferences"));
getDialogPane().getScene().getStylesheets().add(this.getClass().getResource("PreferencesDialog.css").toExternalForm());

Expand All @@ -68,7 +69,7 @@ public PreferencesDialog(JabRefFrame parent) {
close();
});

prefs = JabRefPreferences.getInstance();
prefs = Globals.prefs;
frame = parent;
dialogService = frame.getDialogService();

Expand All @@ -77,7 +78,7 @@ public PreferencesDialog(JabRefFrame parent) {
preferenceTabs.add(new FileTab(dialogService, prefs));
preferenceTabs.add(new TablePrefsTab(prefs));
preferenceTabs.add(new TableColumnsTab(prefs, frame));
preferenceTabs.add(new PreviewPrefsTab(dialogService, ExternalFileTypes.getInstance()));
preferenceTabs.add(new PreviewPrefsTab(dialogService, ExternalFileTypes.getInstance(), taskExecutor));
preferenceTabs.add(new ExternalTab(frame, this, prefs));
preferenceTabs.add(new GroupsPrefsTab(prefs));
preferenceTabs.add(new EntryEditorPrefsTab(prefs));
Expand Down Expand Up @@ -111,8 +112,8 @@ private void construct() {
});
tabsList.getSelectionModel().selectFirst();
new ViewModelListCellFactory<PrefsTab>()
.withText(PrefsTab::getTabName)
.install(tabsList);
.withText(PrefsTab::getTabName)
.install(tabsList);

VBox buttonContainer = new VBox();
buttonContainer.setAlignment(Pos.BOTTOM_LEFT);
Expand All @@ -132,21 +133,19 @@ private void construct() {
resetPreferences.setOnAction(e -> resetPreferences());
resetPreferences.setMaxWidth(Double.MAX_VALUE);
buttonContainer.getChildren().addAll(
importPreferences,
exportPreferences,
showPreferences,
resetPreferences
);
importPreferences,
exportPreferences,
showPreferences,
resetPreferences);

VBox spacer = new VBox();
spacer.setPrefHeight(10.0);
VBox.setVgrow(tabsList, Priority.ALWAYS);
VBox.setVgrow(spacer, Priority.SOMETIMES);
vBox.getChildren().addAll(
tabsList,
spacer,
buttonContainer
);
tabsList,
spacer,
buttonContainer);

container.setLeft(vBox);

Expand All @@ -155,16 +154,16 @@ private void construct() {

private void resetPreferences() {
boolean resetPreferencesConfirmed = dialogService.showConfirmationDialogAndWait(
Localization.lang("Reset preferences"),
Localization.lang("Are you sure you want to reset all settings to default values?"),
Localization.lang("Reset preferences"),
Localization.lang("Cancel"));
Localization.lang("Reset preferences"),
Localization.lang("Are you sure you want to reset all settings to default values?"),
Localization.lang("Reset preferences"),
Localization.lang("Cancel"));
if (resetPreferencesConfirmed) {
try {
prefs.clear();

dialogService.showWarningDialogAndWait(Localization.lang("Reset preferences"),
Localization.lang("You must restart JabRef for this to come into effect."));
Localization.lang("You must restart JabRef for this to come into effect."));
} catch (BackingStoreException ex) {
LOGGER.error("Error while resetting preferences", ex);
dialogService.showErrorDialogAndWait(Localization.lang("Reset preferences"), ex);
Expand All @@ -175,17 +174,17 @@ private void resetPreferences() {

private void importPreferences() {
FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
.addExtensionFilter(StandardFileType.XML)
.withDefaultExtension(StandardFileType.XML)
.withInitialDirectory(prefs.setLastPreferencesExportPath()).build();
.addExtensionFilter(StandardFileType.XML)
.withDefaultExtension(StandardFileType.XML)
.withInitialDirectory(prefs.setLastPreferencesExportPath()).build();

dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(file -> {
try {
prefs.importPreferences(file);
updateAfterPreferenceChanges();

dialogService.showWarningDialogAndWait(Localization.lang("Import preferences"),
Localization.lang("You must restart JabRef for this to come into effect."));
Localization.lang("You must restart JabRef for this to come into effect."));
} catch (JabRefException ex) {
LOGGER.error("Error while importing preferences", ex);
dialogService.showErrorDialogAndWait(Localization.lang("Import preferences"), ex);
Expand Down Expand Up @@ -230,10 +229,10 @@ public void setValues() {

private void exportPreferences() {
FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
.addExtensionFilter(StandardFileType.XML)
.withDefaultExtension(StandardFileType.XML)
.withInitialDirectory(prefs.setLastPreferencesExportPath())
.build();
.addExtensionFilter(StandardFileType.XML)
.withDefaultExtension(StandardFileType.XML)
.withInitialDirectory(prefs.setLastPreferencesExportPath())
.build();

dialogService.showFileSaveDialog(fileDialogConfiguration)
.ifPresent(exportFile -> {
Expand Down
Loading