Skip to content

Commit

Permalink
Fix NPE and not on FX Thread in PreviewPrefs Tabs (#4624)
Browse files Browse the repository at this point in the history 
* Fix NPE and not on FX Thread in PreviewPrefs Tabs


Remove obsolete swing stuff
Create proper javafx binding
Fixes #4621
Fixes #4622

Fix another issue where the Preview stlye is not set correctly

* fix checkstyle

Signed-off-by: Siedlerchr <siedlerkiller@gmail.com>

* fix copying of entry preview

* prevent divide by zero excpetion

* Pass task executor as variable
Use Background Task
add error message
improve binding

* remove empty line
  • Loading branch information
@Siedlerchr
Siedlerchr authored Feb 1, 2019
1 parent 769ebd1 commit 629fcc4
Show file tree
Hide file tree
Showing 7 changed files with 134 additions and 153 deletions.
27 changes: 13 additions & 14 deletions src/main/java/org/jabref/gui/JabRefFrame.java
View file
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
View file
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
8 changes: 6 additions & 2 deletions src/main/java/org/jabref/gui/actions/ShowPreferencesAction.java
View file
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
View file
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

0 comments on commit 629fcc4

Please sign in to comment.