This Bugzilla instance is deprecated, and most Eclipse projects now use GitHub or Eclipse GitLab. Please see the deprecation plan for details.
Bug 421170 - DEL from within the Ctrl + E popup saves dirty editors without prompting
Summary: DEL from within the Ctrl + E popup saves dirty editors without prompting
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.4   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 4.4 M6   Edit
Assignee: Paul Elder CLA
QA Contact: Eric Moffatt CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-06 10:59 EST by Eric Moffatt CLA
Modified: 2014-03-04 12:49 EST (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Moffatt CLA 2013-11-06 10:59:05 EST 
Found this while testing bug 418254. Right now the DEL handling silently closes the editor *without* saving.

To repro:

Open some editors and make one or more dirty
Hit Ctrl + E, arrow down to one of the dirty editors and hit DEL

Currently the result is that the editor is closed but not persisted first.
Comment 1 Eric Moffatt CLA 2013-11-06 11:03:17 EST 
Committed:

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=2ffbff1d40067ce46abb5432c6bc5b76d82e98e9

This silently saves the file before closing it.

There is still a defect here in that trying to *prompt* for the save causes NPE's, likely due to the prompt dialog confusing the shell management. Once we VERIFY this fix in M4 we should determine whether to re-open it for a more complete fix as resources permit.
Comment 2 Eric Moffatt CLA 2013-12-10 14:19:58 EST 
Verified in 4.4.0.I20131209-2000.

Dani / Paul, do you think that we need to do more here or is this such an odd corner case that we can leave it as is (which I'm +1 for).
Comment 3 Dani Megert CLA 2013-12-11 05:37:31 EST 
(In reply to Eric Moffatt from comment #2)
> Verified in 4.4.0.I20131209-2000.
> 
> Dani / Paul, do you think that we need to do more here or is this such an
> odd corner case that we can leave it as is (which I'm +1 for).

I'd prefer a deeper look for two reasons:

1. It's scary for the user if the editor just closes.
2. We might do an unwanted save, e.g. when the same file is open in another
   window.

==> we should simply invoke the normal "close" process.
Comment 4 Dani Megert CLA 2014-01-28 06:41:28 EST 
Adjusted the summary to reflect the remaining issue.
Comment 5 Eric Moffatt CLA 2014-02-14 15:25:13 EST 
Given the time we have left I think the only tweak we have time for is to perhaps disable DEL -> Close for dirty editors. Is this an option ?
Comment 6 Paul Elder CLA 2014-02-18 14:59:19 EST 
Changed to add a Save dialog, if appropriate. And, added a guard to detect editor list disposal (which was leading to NPEs). The list is disposed when the save dialog pops up.

https://git.eclipse.org/r/22183
Comment 7 Dani Megert CLA 2014-02-20 05:26:47 EST 
(In reply to Paul Elder from comment #6)
> Changed to add a Save dialog, if appropriate. And, added a guard to detect
> editor list disposal (which was leading to NPEs). The list is disposed when
> the save dialog pops up.
> 
> https://git.eclipse.org/r/22183

Submitted with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=c72486264793cf734d7e2c74302cf0ff06f64635
Comment 8 Dani Megert CLA 2014-02-20 05:26:59 EST 
.
Comment 9 Paul Webster CLA 2014-03-04 12:49:12 EST 
In 4.4.0.I20140303-2000