Bug 60606 - [Navigator] (data loss) Navigator deletes/moves the wrong file in 3.0M8
Summary: [Navigator] (data loss) Navigator deletes/moves the wrong file in 3.0M8
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows XP
: P1 critical (vote)
Target Milestone: 3.0 RC1   Edit
Assignee: Nick Edgar CLA
QA Contact:
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2004-04-30 13:45 EDT by llewellyn falco CLA
Modified: 2004-06-04 21:29 EDT (History)
3 users (show)

See Also:


Attachments
Patch to BaseSelectionListenerAction to defer selection change while action is running (3.18 KB, patch)
2004-05-21 00:42 EDT, Nick Edgar CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description llewellyn falco CLA 2004-04-30 13:45:53 EDT
In Navagotor, 
Select the 'link with editor' button.
Then open a file (FileICareAbout.java). (double click)
then select a different file (StupidCroft.java) (single click)
then right click and say delete, or move, or hit the delete key.
it will prompt you for - are you sure you want to delete (StupidCroft.java)
say yes.

it will instead delete (FileICareAbout.java).

also, undo will not be allowed.so you have to do some localhistory to revieve 
what you lost. much easier if you know which file you actually deleted)
Comment 1 Nick Edgar CLA 2004-05-03 10:54:32 EDT
I cannot reproduce this in M8 using a clean workspace and following the the
steps above.  Can you provide more detailed instructions?  Is there anything in
the .log file?
Comment 2 John Wiegand CLA 2004-05-04 21:05:28 EDT
I failed to reproduce this problem as well.  Is the project under source code 
control?  Can you reproduce this with a new project?
This should be P1 if reproducible.  Otherwise, need additional confirmations 
and additional info.
Comment 3 llewellyn falco CLA 2004-05-05 14:01:57 EDT
i can reproduce this on a fairly constant basis.
a few things to check.
I知 using the M8 release for windows.
i have the navigator undocked in a separate window.
i tend to have a lot of files open (over 5)
the file i want to delete IS NOT OPEN. (i believe this is curial)
and the link is on
Comment 4 Nick Edgar CLA 2004-05-05 15:31:24 EDT
I'm able to reproduce it.  It's important that the Navigator be a detached view.
Comment 5 Nick Edgar CLA 2004-05-05 15:37:13 EDT
The problem is that the confirmation dialog is parented under the main window's
shell, not the view's shell.  When it is closed, the main window is activated,
activating the editor (which was the last active part in the main window), which
triggers a selection changed in the Navigator due to Link with Editor being on,
which changes the selection in DeleteResourceAction.  DeleteResourceAction asks
itself for the selection again after the confirmation dialog, before doing the
delete.

DeleteResourceAction, and most of the Navigator actions, are given the shell at
creation time.
Comment 6 Nick Edgar CLA 2004-05-14 17:27:09 EDT
I've released a fix whereby it only obtains the selection once, before bringing
up the dialog, and uses that throughout the run.

Need to check other SelectionListenerActions that may have the same problem.
Comment 7 Nick Edgar CLA 2004-05-21 00:40:47 EDT
Also fixed up CopyResourceAction (superclass of MoveResourceAction).
This got released for the M9 candidate, I20040521-0010.
Comment 8 Nick Edgar CLA 2004-05-21 00:42:56 EDT
Created attachment 10927 [details]
Patch to BaseSelectionListenerAction to defer selection change while action is running

Attached a patch to BaseSelectionListenerAction which defers processing a
selection change while the action is running.

I'm holding off on releasing this for M9, but it should go in for RC1, after
further testing with delete, copy, move and other actions.
Comment 9 Nick Edgar CLA 2004-05-21 00:44:36 EDT
Should also change these actions to take a workbench part, or workbench part
site, instead of a shell, and change IWorkbenchPartSite.getShell() to return the
correct shell for a detached view.

This is requires some new API change though.
Comment 10 Nick Edgar CLA 2004-05-21 01:03:37 EDT
Jim, the proposed API change would involve adding a constructor taking an
IWorkbenchPart, IWorkbenchPartSite, or IWorkbenchSite instead of a shell in
most/all of the actions in org.eclipse.ui.ide/extensions/org.eclipse.ui.actions,
and deprecated the existing constructors.  Probably best to use IWorkbenchSite
so that IPage objects can use the actions too.

When the shell is needed by the action, it would be obtained from the site.
PartSite.getShell() would be changed to get the shell from the PartPane's
control, not the window.
Comment 11 Michael Van Meekeren CLA 2004-05-27 10:21:10 EDT
should be 3.0 RC1 not 2.0 RC1
Comment 12 Nick Edgar CLA 2004-05-27 23:10:10 EDT
The patch in comment #8 has been released, which protects other
SelectionListenerActions if they receive a setSelection during a run().
This eliminates the need to fix up other subclasses which query the selection
more than once.

This also reduces the need for new constructors on the actions.
I've filed bug 64486 for this, to be addressed post 3.0.
We'll have to live with the incorrect dialog parenting for detached views for 3.0.
Comment 13 Nick Edgar CLA 2004-06-04 21:29:41 EDT
Verified in I20040604-1600