Fix closing of library not handling backup and autosave manager shutdown #4794

CaptainDaVinci · 2019-03-21T06:59:06Z

The issue was when the tab is closed by clicking 'x' it did not call
the method closeTab() which handles the closing of backup and
autosave manager.

Change in CHANGELOG.md described
Tests created for changes
Manually tested changed features in running JabRef
Screenshots added in PR description (for bigger UI changes)
Ensured that the git commit message is a good one
Check documentation status (Issue created for outdated help page at help.jabref.org?)

Fixes #4789 The issue was when the tab is closed by clicking 'x' it did not call the method `closeTab()` which handles the closing of backup and autosave manager.

tobiasdiez

Thanks for your PR. Let me also take the opportunity to say thanks for your active contribution in other PRs and in the gitter chat!

tobiasdiez · 2019-03-21T23:40:57Z

src/main/java/org/jabref/gui/JabRefFrame.java

@@ -1154,6 +1154,7 @@ public void addTab(BasePanel basePanel, boolean raisePanel) {
            // add tab
            Tab newTab = new Tab(basePanel.getTabTitle(), basePanel);
            tabbedPane.getTabs().add(newTab);
+            newTab.setOnCloseRequest(event -> closeTab((BasePanel) newTab.getContent()));


That's a very good observation.

As of now the user can close the tab even if there are unsaved changes. Thus, we should improve the functionality here a bit by calling confirmClose and consuming the event in case the user canceled the close operation.

The method closeTab() makes a call to confirmClose when the panel is modified. However, it does not consume the event in case of cancelling. Since, the logic for removing tab is already handled I think it should be enough to consume the event in the closeRequest Itself.

The confirmClose shows a dialog with 3 option Save changes, Discard changes and Return to JabRef. The confirmClose method returns false only if the user clicks Return to JabRef in this case, the tab isn't closed however the autosave and backup managers are shutdown any way.
I don't think they should be shutdown in this case, we can return from closeTab() in case confirmClose returns false.

CaptainDaVinci · 2019-03-22T19:23:39Z

Thank you for those kind words! 😄

… to jabref' option

…osing of tab

tobiasdiez

Thanks for the quick follow-up. The changes look good to me!

LinusDietz · 2019-03-26T15:40:45Z

Thank you very much for this fix. Can you please write a Changelog entry, before we merge this?

EDIT: Actually, I'm not sure when this bug was introduced. Probably only recently, so we don't need a changelog entry.

469deb6 Sage Harvard: Add translator/editor 1f51ef8 Add Pensoft dependents (#4981) 5338ff6 Create electrophoresis.csl (#4883) 4de9a97 Update spec_helper.rb fe81f17 Create uirs-urbani-izziv.csl (#4928) 69d37d8 Create uirs-urban-challenge-journal.csl (#4927) d807202 Changes to match Taxon guidelines for authors (#4980) aeb2170 Create ucl-university-college-denmark-apa.csl (#4962) 16f58df Update American Physical Society styles (#4794) b77bd91 Update ucl-university-college-denmark-vancouver.csl (#4963) 81d1a59 Update bibliotheque-d-archeologie-mediterraneenne-et-africaine-biama.csl (#4979) git-subtree-dir: src/main/resources/csl-styles git-subtree-split: 469deb6

fad76fe Update Gemfile.lock a8dafef Update harvard-cite-them-right.csl (JabRef#4993) c8784ff Update and rename emerald-harvard.csl to emerald-harvard-2.csl (JabRef#4991) 197ec0e Update circulation-journal.csl (JabRef#4994) f36dabe Update American Physical Society dependents (JabRef#4992) 93469bb Create journal-of-musculoskeletal-research.csl (JabRef#4990) 43287c7 Update taxon.csl a little more (JabRef#4989) 9f32917 Update and rename university-college-lillebaelt-harvard.csl to ucl-un… (JabRef#4982) 00c3cec Update isnad-metinici.csl (JabRef#4988) 864a64c Update medicine-and-science-in-sports-and-exercise.csl (JabRef#4983) 0c76011 Don't demote particles in Equine Vet J (JabRef#4984) 603ee73 Fix MLA editor translator (JabRef#4985) 40d207c Update pravnik.csl (JabRef#4986) c5fe30c Update isnad-dipnotlu.csl (JabRef#4987) 469deb6 Sage Harvard: Add translator/editor 1f51ef8 Add Pensoft dependents (JabRef#4981) 5338ff6 Create electrophoresis.csl (JabRef#4883) 4de9a97 Update spec_helper.rb fe81f17 Create uirs-urbani-izziv.csl (JabRef#4928) 69d37d8 Create uirs-urban-challenge-journal.csl (JabRef#4927) d807202 Changes to match Taxon guidelines for authors (JabRef#4980) aeb2170 Create ucl-university-college-denmark-apa.csl (JabRef#4962) 16f58df Update American Physical Society styles (JabRef#4794) b77bd91 Update ucl-university-college-denmark-vancouver.csl (JabRef#4963) 81d1a59 Update bibliotheque-d-archeologie-mediterraneenne-et-africaine-biama.csl (JabRef#4979) 0895562 Create pensoft-journals.csl (JabRef#4972) 4f8fa44 creates the gigascience.csl file (JabRef#4961) 1b9c7a0 Create acta-ornithologica.csl (JabRef#4977) c8c6c6d Create sorbonne-student-law-review.csl (JabRef#4956) git-subtree-dir: src/main/resources/csl-styles git-subtree-split: fad76fe

Fix closing of library not handling backup and autosave manager shutdown

54b86ff

Fixes #4789 The issue was when the tab is closed by clicking 'x' it did not call the method `closeTab()` which handles the closing of backup and autosave manager.

tobiasdiez requested changes Mar 21, 2019

View reviewed changes

CaptainDaVinci added 2 commits March 23, 2019 01:15

Prevent closing of tab with unsaved changes when user chooses 'return…

2b6532a

… to jabref' option

Prevent shutdown of autosave and backup managers when user cancels cl…

0eda1c2

…osing of tab

tobiasdiez approved these changes Mar 25, 2019

View reviewed changes

tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 25, 2019

LinusDietz approved these changes Mar 26, 2019

View reviewed changes

LinusDietz merged commit 83c8022 into JabRef:master Mar 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix closing of library not handling backup and autosave manager shutdown #4794

Fix closing of library not handling backup and autosave manager shutdown #4794

CaptainDaVinci commented Mar 21, 2019

tobiasdiez left a comment

tobiasdiez Mar 21, 2019

CaptainDaVinci Mar 22, 2019

CaptainDaVinci Mar 24, 2019

CaptainDaVinci commented Mar 22, 2019

tobiasdiez left a comment

LinusDietz commented Mar 26, 2019 •

edited

Loading

Fix closing of library not handling backup and autosave manager shutdown #4794

Fix closing of library not handling backup and autosave manager shutdown #4794

Conversation

CaptainDaVinci commented Mar 21, 2019

tobiasdiez left a comment

Choose a reason for hiding this comment

tobiasdiez Mar 21, 2019

Choose a reason for hiding this comment

CaptainDaVinci Mar 22, 2019

Choose a reason for hiding this comment

CaptainDaVinci Mar 24, 2019

Choose a reason for hiding this comment

CaptainDaVinci commented Mar 22, 2019

tobiasdiez left a comment

Choose a reason for hiding this comment

LinusDietz commented Mar 26, 2019 • edited Loading

LinusDietz commented Mar 26, 2019 •

edited

Loading