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

Bug 409443

Summary: [sidebar] Commands target wrong item after first unselect?
Product: [ECD] Orion Reporter: Mark Macdonald <mamacdon>
Component: ClientAssignee: Mark Macdonald <mamacdon>
Status: RESOLVED FIXED QA Contact:
Severity: major    
Priority: P3 CC: libingw, Mike_Wilson
Version: 3.0Flags: libingw: review+
Target Milestone: 3.0 RC2   
Hardware: PC   
OS: Windows 7   
Whiteboard:

Description Mark Macdonald CLA 2013-05-29 23:03:07 EDT
1. Open a file in the Orion editor page
2. Select a file in the left-hand navigator pane
3. Reload the page. The file you had selected should be selected automatically.
4. Unselect the file (Ctrl+Click its row, or Cmd+Click on Mac OS)
5. Click [Gear] > Rename

At this point we expect the root of the navigator to be renamed, but sometimes it appears to rename the file that you unselected.

It seems the first time you unselect a file, the unselect gesture doesn't take effect.
Comment 1 Mark Macdonald CLA 2013-06-13 16:34:22 EDT
Detailed explanation:

When you reload the page in Step 3, the sidebar navigator restores the cached selection that you had previously. Hence, your previous file is selected.

A short time later, the editor loads, and the sidebar navigator tries to reveal the file you are editing by selecting it. So it calls
  navHandler.setSelection(fileMetadata)

At this point we run into a problem. The navigator's current selection, and the 'fileMetadata' object loaded by the editor, both represent the same file, but they are 2 different objects. It turns out that when navHandler checks to see if an item is already in the selection, it does a === check:

> inSelection: function(model){
>     for(var i = 0; i < this._selections.length; i++){
>         if(model === this._selections[i]){     // XXX
>             return i;
>         }
>     }
>     return -1;

The === check fails here, even though the 2 objects will actually hash to the same ID using the explorer's FileModel.getId(). Because of this, we end up with the same file selected twice in the selection list, eg:
  [ foo.js, foo.js ]

Now, when you unselect the file using Ctrl+Click, only one of the files is unselected (the one that matches ===). At this point the UI is out of sync with the selection model: the the navigator shows no selection, but the commands will still run against the remaining file in the selection.

The fix is: change the explorerNavHandler to use model.getId() inside _inSelection(), so it correctly detects that the 2 different objects represent the same model item.
Comment 2 Mark Macdonald CLA 2013-06-13 16:34:47 EDT
Here's the patch

http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=5760670
Comment 3 libing wang CLA 2013-06-14 09:22:23 EDT
The fix looks good. Good catch.