From a8d0a18de8503ae768b9041ea2a277c5fb92cd99 Mon Sep 17 00:00:00 2001 From: Adam Wagner Date: Tue, 10 Oct 2017 10:29:42 +0200 Subject: [PATCH] Fix click in subclasses of Grid (#10144) Add findWidget() method to accept non exact matches. --- .../java/com/vaadin/client/WidgetUtil.java | 65 +++++++++++++++++-- .../LegacyLocatorStrategy.java | 6 +- .../client/renderers/ClickableRenderer.java | 2 +- .../com/vaadin/client/ui/VScrollTable.java | 4 +- .../java/com/vaadin/client/ui/VWindow.java | 2 +- .../client/ui/calendar/CalendarConnector.java | 2 +- .../client/ui/dd/VDragAndDropManager.java | 2 +- .../com/vaadin/client/widgets/Escalator.java | 2 +- .../java/com/vaadin/client/widgets/Grid.java | 8 +-- .../tests/components/grid/GridSubclass.java | 11 ++++ .../grid/GridSubclassMouseEvent.java | 18 +++++ .../client/grid/GridSubclassConnector.java | 21 ++++++ .../grid/GridSubclassMouseEventTest.java | 23 +++++++ 13 files changed, 148 insertions(+), 18 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/grid/GridSubclass.java create mode 100644 uitest/src/main/java/com/vaadin/tests/components/grid/GridSubclassMouseEvent.java create mode 100644 uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/GridSubclassConnector.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/grid/GridSubclassMouseEventTest.java 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 @@ -837,6 +837,25 @@ public class WidgetUtil { } }-*/; + /** + * Helper method to find first instance of any Widget found by traversing + * DOM upwards from given element. + *

+ * Note: If {@code element} is inside some widget {@code W} + * , and {@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 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. @@ -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). + *

+ * 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 findWidget(Element element, Class class1) { + return findWidget(element, class1, true); + } + + /** + * Helper method to find first instance of given Widget type found by + * traversing DOM upwards from given element. + *

+ * Note: If {@code element} is inside some widget {@code W} + * , and {@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 findWidget(Element element, + Class 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 * @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 extends ResizeComposite implements HasSelectionHandlers, 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 extends ResizeComposite implements HasSelectionHandlers, 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 extends ResizeComposite implements HasSelectionHandlers, 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 extends ResizeComposite implements HasSelectionHandlers, } 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 { + + } + + @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()); + } +} -- 2.39.5