Fix selecting rows from header and footer (#10720)

3 files changed, 47 insertions, 5 deletions

diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java
index 8a6d7abfdd..1d24086b3b 100755
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -26,13 +26,14 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Stream;
 
 import com.google.gwt.core.client.Scheduler;
 import com.google.gwt.core.client.Scheduler.ScheduledCommand;
@@ -2403,8 +2404,15 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             EventTarget target = getNativeEvent().getEventTarget();
             Grid<?> grid = getGrid();
             if (Element.is(target) && grid != null) {
+                final RowContainer container = Stream
+                        .of(grid.escalator.getHeader(),
+                                grid.escalator.getBody(),
+                                grid.escalator.getFooter())
+                        .filter(c -> c.getCell(target.cast()) != null)
+                        .findFirst()
+                        .orElse(grid.cellFocusHandler.containerWithFocus);
+
                 Section section = Section.FOOTER;
-                final RowContainer container = grid.cellFocusHandler.containerWithFocus;
                 if (container == grid.escalator.getHeader()) {
                     section = Section.HEADER;
                 } else if (container == getGrid().escalator.getBody()) {
@@ -8640,7 +8648,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
      * Adds a spacer visibility changed handler to the underlying escalator.
      *
      * @param handler
-     *         the handler to be called when a spacer's visibility changes
+     *            the handler to be called when a spacer's visibility changes
      * @return the registration object with which the handler can be removed
      * @since 8.3.2
      */
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponents.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponents.java
index 97437fbbd2..d4b76a2722 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponents.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponents.java
@@ -76,6 +76,7 @@ public class GridComponents extends AbstractTestUIWithLog {
         resetData.click();
         addComponent(resetData);
 
+        grid.appendHeaderRow().join("label", "textField", "button").setComponent(new TextField());
+        grid.appendFooterRow().join("label", "textField", "button").setComponent(new TextField());
     }
-
 }
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java
index b9cc1fef35..8948bdfb68 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java
@@ -13,12 +13,15 @@ import org.openqa.selenium.Keys;
 import org.openqa.selenium.WebElement;
 
 import com.vaadin.testbench.By;
+import com.vaadin.testbench.annotations.RunLocally;
 import com.vaadin.testbench.elements.ButtonElement;
 import com.vaadin.testbench.elements.GridElement;
 import com.vaadin.testbench.elements.GridElement.GridCellElement;
 import com.vaadin.testbench.elements.GridElement.GridRowElement;
 import com.vaadin.testbench.elements.LabelElement;
 import com.vaadin.testbench.elements.NotificationElement;
+import com.vaadin.testbench.elements.TextFieldElement;
+import com.vaadin.testbench.parallel.Browser;
 import com.vaadin.testbench.parallel.BrowserUtil;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 
@@ -153,6 +156,35 @@ public class GridComponentsTest extends MultiBrowserTest {
         assertTrue("Row should be selected", grid.getRow(0).isSelected());
     }
 
+    @Test
+    public void testRowNotSelectedFromHeaderOrFooter() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+        grid.getCell(4, 0).$(LabelElement.class).first().click(10, 10,
+                new Keys[0]);
+        assertTrue("Row 4 should be selected", grid.getRow(4).isSelected());
+
+        TextFieldElement headerTextField = grid.getHeaderCell(1, 0)
+                .$(TextFieldElement.class).first();
+        headerTextField.sendKeys(Keys.SPACE);
+
+        assertFalse("Row 1 should not be selected",
+                grid.getRow(1).isSelected());
+        assertTrue("Row 4 should still be selected",
+                grid.getRow(4).isSelected());
+
+        TextFieldElement footerTextField = grid.getFooterCell(0, 0)
+                .$(TextFieldElement.class).first();
+        footerTextField.sendKeys(Keys.SPACE);
+
+        assertFalse("Row 0 should not be selected",
+                grid.getRow(0).isSelected());
+        assertTrue("Row 4 should still be selected",
+                grid.getRow(4).isSelected());
+
+    }
+
     private void assertRowExists(int i, String string) {
         GridRowElement row = $(GridElement.class).first().getRow(i);
         assertEquals("Label text did not match", string,
@@ -162,9 +194,10 @@ public class GridComponentsTest extends MultiBrowserTest {
                 .click();
         // IE 11 is slow, need to wait for the notification.
         waitUntil(driver -> isElementPresent(NotificationElement.class), 10);
-        assertTrue("Notification should contain given text",
+        assertTrue("Notification should contain given text: " + string,
                 $(NotificationElement.class).first().getText()
                         .contains(string));
+        $(NotificationElement.class).first().close();
     }
 
     private void assertNoButton(int i) {