From 0397abb05ca26e7dc4cd5e976c7449cb5f61ac18 Mon Sep 17 00:00:00 2001
From: Akos Kitta <kittaakos@typefox.io>
Date: Mon, 18 Mar 2019 10:45:52 +0100
Subject: [PATCH] GH-4585: Fixed the selection state issue.

From now on, the nodes are remapped from the `tree`,
to ensure the `selection` and `focus` represents the most recent state.
This fix ensures that the selection state does not get out of sync.

Closes #4585.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
---
 .../src/browser/tree/tree-selection-state.ts  | 37 ++++++++++++++-----
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/packages/core/src/browser/tree/tree-selection-state.ts b/packages/core/src/browser/tree/tree-selection-state.ts
index 8f56c1158f697..045cfe7f1337c 100644
--- a/packages/core/src/browser/tree/tree-selection-state.ts
+++ b/packages/core/src/browser/tree/tree-selection-state.ts
@@ -14,7 +14,7 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-import { Tree } from './tree';
+import { Tree, TreeNode } from './tree';
 import { DepthFirstTreeIterator } from './tree-iterator';
 import { TreeSelection, SelectableTreeNode } from './tree-selection';
 
@@ -74,26 +74,29 @@ export class TreeSelectionState {
 
     selection(): ReadonlyArray<SelectableTreeNode> {
         const copy = this.checkNoDefaultSelection(this.selectionStack);
-        const nodes = new Set();
+        const nodeIds = new Set<string>();
         for (let i = 0; i < copy.length; i++) {
             const { node, type } = copy[i];
             if (TreeSelection.isRange(type)) {
                 const selection = copy[i];
-                this.selectionRange(selection).forEach(n => nodes.add(n));
+                for (const id of this.selectionRange(selection).map(n => n.id)) {
+                    nodeIds.add(id);
+                }
             } else if (TreeSelection.isToggle(type)) {
-                if (nodes.has(node)) {
-                    nodes.delete(node);
+                if (nodeIds.has(node.id)) {
+                    nodeIds.delete(node.id);
                 } else {
-                    nodes.add(node);
+                    nodeIds.add(node.id);
                 }
             }
         }
-        return Array.from(nodes.keys()).reverse();
+        return Array.from(nodeIds.keys()).map(id => this.tree.getNode(id)).filter(SelectableTreeNode.is).reverse();
     }
 
     get focus(): SelectableTreeNode | undefined {
         const copy = this.checkNoDefaultSelection(this.selectionStack);
-        return copy[copy.length - 1].focus;
+        const candidate = copy[copy.length - 1].focus;
+        return this.toSelectableTreeNode(candidate);
     }
 
     protected handleDefault(state: TreeSelectionState, node: Readonly<SelectableTreeNode>): TreeSelectionState {
@@ -139,7 +142,7 @@ export class TreeSelectionState {
         // Drop the previous range when we are trying to modify that.
         if (TreeSelection.isRange(copy[copy.length - 1])) {
             const range = this.selectionRange(copy.pop()!);
-            // And we drop all preceeding individual nodes that were contained in the range we are dropping.
+            // And we drop all preceding individual nodes that were contained in the range we are dropping.
             // That means, anytime we cover individual nodes with a range, they will belong to the range so we need to drop them now.
             for (let i = copy.length - 1; i >= 0; i--) {
                 if (range.indexOf(copy[i].node) !== -1) {
@@ -208,6 +211,22 @@ export class TreeSelectionState {
         return range.filter(SelectableTreeNode.is);
     }
 
+    protected toSelectableTreeNode(node: TreeNode | undefined): SelectableTreeNode | undefined {
+        if (!!node) {
+            const candidate = this.tree.getNode(node.id);
+            if (!!candidate) {
+                if (SelectableTreeNode.is(candidate)) {
+                    return candidate;
+                } else {
+                    console.warn(`Could not map to a selectable tree node. Node with ID: ${node.id} is not a selectable node.`);
+                }
+            } else {
+                console.warn(`Could not map to a selectable tree node. Node does not exist with ID: ${node.id}.`);
+            }
+        }
+        return undefined;
+    }
+
     /**
      * Checks whether the argument contains any `DEFAULT` tree selection type. If yes, throws an error, otherwise returns with a reference the argument.
      */