Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 74028

Summary:	[View Mgmt] Save confirmation dialog pops up when close ISaveablePart view
Product:	[Eclipse Project] Platform	Reporter:	Amy Wu <for.work.things>
Component:	UI	Assignee:	Nick Edgar <n.a.edgar>
Status:	VERIFIED FIXED	QA Contact:
Severity:	normal
Priority:	P2	CC:	ahunter.eclipse, dleroux, steven.wasleski
Version:	3.0	Keywords:	usability
Target Milestone:	3.1 M5
Hardware:	PC
OS:	Windows XP
Whiteboard:

Description Amy Wu

2004-09-15 18:14:37 EDT

using M20040908, i think.

I just noticed Bug 72114 - Views that implement ISaveablePart are not 
consulted for dirty state before they are closed.  While I think this is 
pretty nice, I think the views should also be consulted to see if they even 
want to throw up the save on close confirmation dialog.  I saw there was a 
comment in bug 72114:

------- Additional Comment #5 From Nick Edgar 2004-08-31 14:56 ------- 
Actually, in order to avoid unwanted change in behaviour in 3.0.1 for other
views implementing ISaveablePart, we should check an internal preference as to
whether to prompt on dirty view close.  Default would be false.

Does this internal preference exist and is that what views can use to say they 
dont want to confirm save on close?  Because like the comment mentions above, 
I am one of those people who consider this an unwanted change in behaviour for 
3.0.1.

My bottom line is that I would like a way for my view to say when it wants the 
save on close confirm dialog or not.  From what I can see, views are hard-
coded to always have the dialog (if the view isDirty) according to 
org.eclipse.ui.internal.Perspective.canCloseView.

FYI, my situation is that my view is basically used for script editing.  (I've 
explained my situation before in bug 45248) So it essentially contains an 
editor within the view.  Sometimes the script being edited is contained in the 
active editor, other times it's not.  I needed to use ISaveablePart so that 
when you save in my view, the code in my view is saved, meaning maybe the 
active editor was saved, maybe some other editor was saved.  I feel that 
adding the save on close confirmation dialog would confuse users because if 
the user is currently editing code that's in both the active editor and in my 
view, there really isnt a need to save before closing my view.  Plus the name 
of the view that shows up in the dialog is a pretty confusing because I dont 
currently display the name of the file being edited in my view.

Comment 1 Nick Edgar

2004-09-15 21:20:03 EDT

Interesting use case.  This argues for giving the view more explicit control
over how/when saving is done, including possibly the prompt dialog, or at least
the name shown.

For bug 72114, I was hoping that we would be able to just get away with the
change in behaviour, but that was apparently too optimistic.  I should have
stayed with my original, crusty, conservative outlook <g>.

To do this properly, we'll have to define new API, which isn't possible at this
point for 3.0.1.  I'll add the preference as I originally suggested, with the
default to give the 3.0 behaviour (no prompt). 

Actually, with the existing API, there's enough there to determine whether to
save on close at all.  See ISaveablePart.isSaveOnCloseNeeded().
That won't address the problem with using the view name though.

Comment 2 Amy Wu

2004-09-16 01:17:46 EDT

The preference key along with the check for ISaveablePart.isSaveOnCloseNeeded
() is a good enough fix for me for v3.0.1 (with the view name issue to be 
addressed later).  Thanks for the quick response.

So for now, the save on close dialog will not even pop up unless we have the 
preference key specified in the plugin_customization.ini right?

Comment 3 Nick Edgar

2004-09-16 10:53:00 EDT

Correct.  Do you think this is something that RAD will likely use?

Comment 4 Amy Wu

2004-09-16 16:47:17 EDT

Actually, I'm not familiar with anyone else in RAD using ISaveablePart except 
for me.  So speaking for myself, for 3.0.1, I'd like to not use the dialog and 
so by not even specifying the preference key, everything should be fine.

For a future release, I'd like to take better advantage of ISaveablePart and 
this confirmation dialog.  For example, right now, I almost always have isDirty
() return true, just so the Save action is always enabled.  I'd like to 
improve my isDirty to more accuratly reflect whether or not a file is dirty in 
my view.  And also for the confirmation dialog I would probably check first if 
my file is already opened in another editor, if so, I don't need the 
confirmation dialog.

Comment 5 Nick Edgar

2004-09-16 17:35:42 EDT

With the fix for 72114, isDirty() and isSaveOnCloseNeeded() should give you the
flexibility you need, except for it using the view name in the prompt.

Comment 6 Nick Edgar

2004-10-15 09:42:12 EDT

Want to resolve this for 3.1.  Dan, can you comment on your interest in this?

Comment 7 Dan Leroux

2004-10-15 13:24:51 EDT

This along with 60528 and 72144 are required to fix one of the biggest 
usability issues with RSM/RSA...I would suggest higher priority.

Comment 8 Nick Edgar

2004-10-15 14:51:13 EDT

OK.

Comment 9 Nick Edgar

2005-02-07 17:58:10 EST

I've taken out the fix72114 preference check from Perspective.canCloseView().
So the code is now:

    public boolean canCloseView(IViewPart view) {
		if (view instanceof ISaveablePart) {
			ISaveablePart saveable = (ISaveablePart)view;
			if (saveable.isSaveOnCloseNeeded()) {
				IWorkbenchWindow window = view.getSite().getWorkbenchWindow();		
				return SaveableHelper.savePart(saveable, view, window, true);
			}
		}
    	return true;
    }

If the view doesn't want the prompt to save when dirty, it can return false for
isSaveOnCloseNeeded().

For customizing the appearance of the save on close prompt, see bug 76768.

For Save All support, and other ISaveablePart issues, see bug 84406.

Comment 10 Nick Edgar

2005-02-16 15:53:12 EST

Verified in I20040215-2300.