Fix click in subclasses of Grid (#10144)

Add findWidget() method to accept non exact matches.

13 files changed, 148 insertions, 18 deletions

diff --git a/client/src/main/java/com/vaadin/client/WidgetUtil.java b/client/src/main/java/com/vaadin/client/WidgetUtil.java
index 8ce2e0c500..f256fc53f2 100644
--- a/client/src/main/java/com/vaadin/client/WidgetUtil.java
+++ b/client/src/main/java/com/vaadin/client/WidgetUtil.java
@@ -838,6 +838,25 @@ public class WidgetUtil {
     }-*/;
 
     /**
+     * Helper method to find first instance of any Widget found by traversing
+     * DOM upwards from given element.
+     * <p>
+     * <strong>Note:</strong> If {@code element} is inside some widget {@code W}
+     * , <em>and</em> {@code W} in turn is wrapped in a {@link Composite}
+     * {@code C}, this method will not find {@code W} but returns {@code C}.
+     * This may also be the case with other Composite-like classes that hijack
+     * the event handling of their child widget(s).
+     *
+     * @param element
+     *            the element where to start seeking of Widget
+     * @since 7.7.11
+     */
+    @SuppressWarnings("unchecked")
+    public static <T> T findWidget(Element element) {
+        return findWidget(element, null);
+    }
+
+    /**
      * Helper method to find first instance of given Widget type found by
      * traversing DOM upwards from given element.
      * <p>
@@ -847,15 +866,43 @@ public class WidgetUtil {
      * {@code C} or null, depending on whether the class parameter matches. This
      * may also be the case with other Composite-like classes that hijack the
      * event handling of their child widget(s).
+     * <p>
+     * Only accepts the exact class {@code class1} if not null.
      *
      * @param element
      *            the element where to start seeking of Widget
      * @param class1
-     *            the Widget type to seek for
+     *            the Widget type to seek for, null for any
      */
     @SuppressWarnings("unchecked")
     public static <T> T findWidget(Element element,
             Class<? extends Widget> class1) {
+        return findWidget(element, class1, true);
+    }
+
+    /**
+     * Helper method to find first instance of given Widget type found by
+     * traversing DOM upwards from given element.
+     * <p>
+     * <strong>Note:</strong> If {@code element} is inside some widget {@code W}
+     * , <em>and</em> {@code W} in turn is wrapped in a {@link Composite} {@code
+     * C}, this method will not find {@code W}. It returns either {@code C} or
+     * null, depending on whether the class parameter matches. This may also be
+     * the case with other Composite-like classes that hijack the event handling
+     * of their child widget(s).
+     *
+     * @param element
+     *            the element where to start seeking of Widget
+     * @param class1
+     *            the Widget type to seek for
+     * @param exactMatch
+     *            true to only accept class1, false to also accept its
+     *            superclasses
+     * @since 7.7.11
+     */
+    @SuppressWarnings("unchecked")
+    public static <T> T findWidget(Element element,
+            Class<? extends Widget> class1, boolean exactMatch) {
         if (element != null) {
             /* First seek for the first EventListener (~Widget) from dom */
             EventListener eventListener = null;
@@ -871,9 +918,19 @@ public class WidgetUtil {
                  * hierarchy
                  */
                 Widget w = (Widget) eventListener;
+                if (class1 == null && w != null) {
+                    return (T) w;
+                }
                 while (w != null) {
-                    if (class1 == null || w.getClass() == class1) {
-                        return (T) w;
+                    Class<?> widgetClass = w.getClass();
+                    while (widgetClass != null) {
+                        if (widgetClass == class1) {
+                            return (T) w;
+                        }
+                        // terminate after first check if looking for exact
+                        // match
+                        widgetClass = exactMatch ? null
+                                : widgetClass.getSuperclass();
                     }
                     w = w.getParent();
                 }
@@ -1110,7 +1167,7 @@ public class WidgetUtil {
          * Fixes infocusable form fields in Safari of iOS 5.x and some Android
          * browsers.
          */
-        Widget targetWidget = findWidget(target, null);
+        Widget targetWidget = findWidget(target);
         if (targetWidget instanceof com.google.gwt.user.client.ui.Focusable) {
             final com.google.gwt.user.client.ui.Focusable toBeFocusedWidget = (com.google.gwt.user.client.ui.Focusable) targetWidget;
             toBeFocusedWidget.setFocus(true);
diff --git a/client/src/main/java/com/vaadin/client/componentlocator/LegacyLocatorStrategy.java b/client/src/main/java/com/vaadin/client/componentlocator/LegacyLocatorStrategy.java
index 9e36e18162..7e8df231f8 100644
--- a/client/src/main/java/com/vaadin/client/componentlocator/LegacyLocatorStrategy.java
+++ b/client/src/main/java/com/vaadin/client/componentlocator/LegacyLocatorStrategy.java
@@ -213,7 +213,7 @@ public class LegacyLocatorStrategy implements LocatorStrategy {
         // widget to which the path is relative. Otherwise, the current
         // implementation simply interprets the path as if baseElement was
         // null.
-        Widget baseWidget = WidgetUtil.findWidget(baseElement, null);
+        Widget baseWidget = WidgetUtil.findWidget(baseElement);
 
         Widget w = getWidgetFromPath(widgetPath, baseWidget);
         if (w == null || !WidgetUtil.isAttachedAndDisplayed(w)) {
@@ -337,8 +337,8 @@ public class LegacyLocatorStrategy implements LocatorStrategy {
                 String childIndexString = part.substring("domChild[".length(),
                         part.length() - 1);
 
-                if (WidgetUtil.findWidget(baseElement,
-                        null) instanceof VAbstractOrderedLayout) {
+                if (WidgetUtil.findWidget(baseElement)
+                        instanceof VAbstractOrderedLayout) {
                     if (element.hasChildNodes()) {
                         Element e = element.getFirstChildElement().cast();
                         String cn = e.getClassName();
diff --git a/client/src/main/java/com/vaadin/client/renderers/ClickableRenderer.java b/client/src/main/java/com/vaadin/client/renderers/ClickableRenderer.java
index a79e541a93..9ed32f0693 100644
--- a/client/src/main/java/com/vaadin/client/renderers/ClickableRenderer.java
+++ b/client/src/main/java/com/vaadin/client/renderers/ClickableRenderer.java
@@ -180,7 +180,7 @@ public abstract class ClickableRenderer<T, W extends Widget>
          * @return the parent grid or null if none found.
          */
         private static Grid<?> findClosestParentGrid(Element e) {
-            Widget w = WidgetUtil.findWidget(e, null);
+            Widget w = WidgetUtil.findWidget(e);
 
             while (w != null && !(w instanceof Grid)) {
                 w = w.getParent();
diff --git a/client/src/main/java/com/vaadin/client/ui/VScrollTable.java b/client/src/main/java/com/vaadin/client/ui/VScrollTable.java
index cd84e83ebe..401ea15ac2 100644
--- a/client/src/main/java/com/vaadin/client/ui/VScrollTable.java
+++ b/client/src/main/java/com/vaadin/client/ui/VScrollTable.java
@@ -6473,7 +6473,7 @@ public class VScrollTable extends FlowPanel
 
             private Element getElementTdOrTr(Element element) {
 
-                Widget widget = WidgetUtil.findWidget(element, null);
+                Widget widget = WidgetUtil.findWidget(element);
 
                 if (widget != this) {
                     /*
@@ -8339,7 +8339,7 @@ public class VScrollTable extends FlowPanel
     @Override
     public String getSubPartName(
             com.google.gwt.user.client.Element subElement) {
-        Widget widget = WidgetUtil.findWidget(subElement, null);
+        Widget widget = WidgetUtil.findWidget(subElement);
         if (widget instanceof HeaderCell) {
             return SUBPART_HEADER + "[" + tHead.visibleCells.indexOf(widget)
                     + "]";
diff --git a/client/src/main/java/com/vaadin/client/ui/VWindow.java b/client/src/main/java/com/vaadin/client/ui/VWindow.java
index caee423965..cfdaecdb90 100644
--- a/client/src/main/java/com/vaadin/client/ui/VWindow.java
+++ b/client/src/main/java/com/vaadin/client/ui/VWindow.java
@@ -1323,7 +1323,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner,
             if (!DOM.isOrHasChild(getTopmostWindow().getElement(), target)) {
                 // not within the modal window, but let's see if it's in the
                 // debug window
-                Widget w = WidgetUtil.findWidget(target, null);
+                Widget w = WidgetUtil.findWidget(target);
                 while (w != null) {
                     if (w instanceof VDebugWindow) {
                         return true; // allow debug-window clicks
diff --git a/client/src/main/java/com/vaadin/client/ui/calendar/CalendarConnector.java b/client/src/main/java/com/vaadin/client/ui/calendar/CalendarConnector.java
index 04c6943edc..0123ee1b3a 100644
--- a/client/src/main/java/com/vaadin/client/ui/calendar/CalendarConnector.java
+++ b/client/src/main/java/com/vaadin/client/ui/calendar/CalendarConnector.java
@@ -428,7 +428,7 @@ public class CalendarConnector extends AbstractComponentConnector
     public TooltipInfo getTooltipInfo(
             com.google.gwt.dom.client.Element element) {
         TooltipInfo tooltipInfo = null;
-        Widget w = WidgetUtil.findWidget(element, null);
+        Widget w = WidgetUtil.findWidget(element);
         if (w instanceof HasTooltipKey) {
             tooltipInfo = GWT.create(TooltipInfo.class);
             String title = tooltips.get(((HasTooltipKey) w).getTooltipKey());
diff --git a/client/src/main/java/com/vaadin/client/ui/dd/VDragAndDropManager.java b/client/src/main/java/com/vaadin/client/ui/dd/VDragAndDropManager.java
index a3e3e9bcf1..15e915334d 100644
--- a/client/src/main/java/com/vaadin/client/ui/dd/VDragAndDropManager.java
+++ b/client/src/main/java/com/vaadin/client/ui/dd/VDragAndDropManager.java
@@ -412,7 +412,7 @@ public class VDragAndDropManager {
      */
     protected VDropHandler findDragTarget(Element element) {
         try {
-            Widget w = WidgetUtil.findWidget(element, null);
+            Widget w = WidgetUtil.findWidget(element);
             if (w == null) {
                 return null;
             }
diff --git a/client/src/main/java/com/vaadin/client/widgets/Escalator.java b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
index 898379106e..7b4add7661 100644
--- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
@@ -6450,7 +6450,7 @@ public class Escalator extends Widget
             @SuppressWarnings("deprecation")
             com.google.gwt.user.client.Element castElement = (com.google.gwt.user.client.Element) possibleWidgetNode
                     .cast();
-            Widget w = WidgetUtil.findWidget(castElement, null);
+            Widget w = WidgetUtil.findWidget(castElement);
 
             // Ensure findWidget did not traverse past the cell element in the
             // DOM hierarchy
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 21d7d0ae5d..daf6974ada 100755
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -2344,7 +2344,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             if (!Element.is(target)) {
                 return null;
             }
-            return WidgetUtil.findWidget(Element.as(target), Grid.class);
+            return WidgetUtil.findWidget(Element.as(target), Grid.class, false);
         }
 
         /**
@@ -2411,7 +2411,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             if (!Element.is(target)) {
                 return null;
             }
-            return WidgetUtil.findWidget(Element.as(target), Grid.class);
+            return WidgetUtil.findWidget(Element.as(target), Grid.class, false);
         }
 
         /**
@@ -5671,7 +5671,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
                 if (renderer instanceof WidgetRenderer) {
                     try {
                         Widget w = WidgetUtil.findWidget(
-                                cell.getElement().getFirstChildElement(), null);
+                                cell.getElement().getFirstChildElement());
                         if (w != null) {
 
                             // Logical detach
@@ -7482,7 +7482,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
     }
 
     private boolean isElementInChildWidget(Element e) {
-        Widget w = WidgetUtil.findWidget(e, null);
+        Widget w = WidgetUtil.findWidget(e);
 
         if (w == this) {
             return false;
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSubclass.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSubclass.java
new file mode 100644
index 0000000000..eddaaa38e2
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSubclass.java
@@ -0,0 +1,11 @@
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.data.Container.Indexed;
+import com.vaadin.ui.Grid;
+
+public class GridSubclass extends Grid {
+
+    public GridSubclass(Indexed dataSource) {
+        super(dataSource);
+    }
+}
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSubclassMouseEvent.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSubclassMouseEvent.java
new file mode 100644
index 0000000000..4f1cfbdab6
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSubclassMouseEvent.java
@@ -0,0 +1,18 @@
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.tests.util.PersonContainer;
+import com.vaadin.tests.widgetset.TestingWidgetSet;
+
+@Widgetset(TestingWidgetSet.NAME)
+public class GridSubclassMouseEvent extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        PersonContainer container = PersonContainer.createWithTestData();
+        GridSubclass grid = new GridSubclass(container);
+        addComponent(grid);
+    }
+}
diff --git a/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/GridSubclassConnector.java b/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/GridSubclassConnector.java
new file mode 100644
index 0000000000..9d8a5d65aa
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/GridSubclassConnector.java
@@ -0,0 +1,21 @@
+package com.vaadin.tests.widgetset.client.grid;
+
+import com.vaadin.client.connectors.GridConnector;
+import com.vaadin.client.widgets.Grid;
+import com.vaadin.shared.ui.Connect;
+import com.vaadin.tests.components.grid.GridSubclass;
+
+import elemental.json.JsonObject;
+
+@Connect(GridSubclass.class)
+public class GridSubclassConnector extends GridConnector {
+
+    public static class GridSubclass extends Grid<JsonObject> {
+
+    }
+
+    @Override
+    public GridSubclass getWidget() {
+        return (GridSubclass) super.getWidget();
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSubclassMouseEventTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSubclassMouseEventTest.java
new file mode 100644
index 0000000000..128abe37e6
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSubclassMouseEventTest.java
@@ -0,0 +1,23 @@
+package com.vaadin.tests.components.grid;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.testbench.elements.GridElement.GridRowElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class GridSubclassMouseEventTest extends SingleBrowserTest {
+
+    @Test
+    public void selectRowWithMouseClick() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+
+        GridRowElement row = grid.getRow(0);
+        Assert.assertTrue("Row should not be selected", !row.isSelected());
+        grid.getCell(0, 0).click();
+        Assert.assertTrue("Row should be selected", row.isSelected());
+    }
+}