Fixes right click selection focus issues in Tree. (#12618)

Change-Id: I1057424fc8956b05e15a92c32e48a767e9fc8df9

3 files changed, 140 insertions, 2 deletions

diff --git a/client/src/com/vaadin/client/ui/tree/TreeConnector.java b/client/src/com/vaadin/client/ui/tree/TreeConnector.java
index ef016c31b7..7560a0f56b 100644
--- a/client/src/com/vaadin/client/ui/tree/TreeConnector.java
+++ b/client/src/com/vaadin/client/ui/tree/TreeConnector.java
@@ -18,6 +18,7 @@ package com.vaadin.client.ui.tree;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.Set;
 
 import com.google.gwt.aria.client.Roles;
 import com.google.gwt.dom.client.Element;
@@ -136,9 +137,24 @@ public class TreeConnector extends AbstractComponentConnector implements
             getWidget().lastSelection = getWidget().getNodeByKey(
                     getWidget().lastSelection.key);
         }
+
         if (getWidget().focusedNode != null) {
-            getWidget().setFocusedNode(
-                    getWidget().getNodeByKey(getWidget().focusedNode.key));
+
+            Set<String> selectedIds = getWidget().selectedIds;
+
+            // If the focused node is not between the selected nodes, we need to
+            // refresh the focused node to prevent an undesired scroll. #12618.
+            if (!selectedIds.isEmpty()
+                    && !selectedIds.contains(getWidget().focusedNode.key)) {
+                String keySelectedId = selectedIds.iterator().next();
+
+                TreeNode nodeToSelect = getWidget().getNodeByKey(keySelectedId);
+
+                getWidget().setFocusedNode(nodeToSelect);
+            } else {
+                getWidget().setFocusedNode(
+                        getWidget().getNodeByKey(getWidget().focusedNode.key));
+            }
         }
 
         if (getWidget().lastSelection == null
diff --git a/uitest/src/com/vaadin/tests/components/tree/TreeScrollingOnRightClick.java b/uitest/src/com/vaadin/tests/components/tree/TreeScrollingOnRightClick.java
new file mode 100644
index 0000000000..8a2b263006
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/tree/TreeScrollingOnRightClick.java
@@ -0,0 +1,53 @@
+package com.vaadin.tests.components.tree;
+
+import com.vaadin.event.ItemClickEvent;
+import com.vaadin.event.MouseEvents;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Tree;
+
+/**
+ * Test for #12618: Trying to select item with right click in Tree causes focus
+ * issues.
+ */
+@SuppressWarnings("serial")
+public class TreeScrollingOnRightClick extends AbstractTestUI {
+
+    public static final String TREE_ID = "my-tree";
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final Tree tree = new Tree();
+        tree.setId(TREE_ID);
+        tree.setSizeUndefined();
+
+        // Add item click listener for right click selection
+        tree.addItemClickListener(new ItemClickEvent.ItemClickListener() {
+            @SuppressWarnings("deprecation")
+            @Override
+            public void itemClick(ItemClickEvent event) {
+                if (event.getButton() == MouseEvents.ClickEvent.BUTTON_RIGHT) {
+                    tree.select(event.getItemId());
+                }
+            }
+        });
+
+        // Add some items
+        for (int i = 0; i < 200; i++) {
+            tree.addItem(String.format("Node %s", i));
+        }
+
+        addComponent(tree);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Right clicking on items should not scroll Tree.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 12618;
+    }
+
+}
diff --git a/uitest/src/com/vaadin/tests/components/tree/TreeScrollingOnRightClickTest.java b/uitest/src/com/vaadin/tests/components/tree/TreeScrollingOnRightClickTest.java
new file mode 100644
index 0000000000..76ab1b3fdb
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/tree/TreeScrollingOnRightClickTest.java
@@ -0,0 +1,69 @@
+/* 
+ * Copyright 2000-2013 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.tree;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.Point;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.interactions.Actions;
+
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * 
+ * @since 7.1.9
+ * @author Vaadin Ltd
+ */
+public class TreeScrollingOnRightClickTest extends MultiBrowserTest {
+
+    @Test
+    public void testScrollingOnRightClick() throws Throwable {
+        openTestURL();
+
+        // Focus tree
+        WebElement tree = getDriver().findElement(
+                By.id(TreeScrollingOnRightClick.TREE_ID));
+        tree.click();
+
+        // Move selection down 50 items
+        for (int down = 0; down < 50; down++) {
+            tree.sendKeys(Keys.ARROW_DOWN);
+        }
+
+        Thread.sleep(1000);
+
+        // Get location of item 40
+        Point item40Location = getTreeNode("Node 40").getLocation();
+
+        // Right click on item 45
+        WebElement item45 = getTreeNode("Node 45");
+        new Actions(getDriver()).moveToElement(item45).contextClick(item45)
+                .perform();
+
+        // Ensure location of item 40 is still the same (no scrolling)
+        Point item40Location2 = getTreeNode("Node 40").getLocation();
+        assertEquals(item40Location.getY(), item40Location2.getY());
+    }
+
+    private WebElement getTreeNode(String caption) {
+        return getDriver().findElement(
+                By.xpath("//span[text() = '" + caption + "']"));
+    }
+}