| Summary: | [sidebar] Commands target wrong item after first unselect? | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Mark Macdonald <mamacdon> |
| Component: | Client | Assignee: | Mark Macdonald <mamacdon> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P3 | CC: | libingw, Mike_Wilson |
| Version: | 3.0 | Flags: | libingw:
review+
|
| Target Milestone: | 3.0 RC2 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
|
Description
Mark Macdonald
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.
The fix looks good. Good catch. |