Fix Table column header sorting on Chrome (#14796)

This fix is similar to the fix that has been made for other similar
cases (i.e. #13381).
Couldn't find a reliable way to reproduce the problem. Hopefully this
will fix the issue.

Was reproduced (before fix) on Google Chrome 40.0.2214.115 m on
TableSortingStopsWorkingOnChrome test one time (but then suddenly it
started to work again).
Was reproduced (before fix) on Project TableSorting once, as described
in the ticket. That project has been attached to the ticket.

Change-Id: Id901c9ce4a0a7c369572bf4374223851658aa703

diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java
index 12de2724fc429ba1547347d384ce9d843a79dc5b..13561dcd0f3922d3a15bdebe812cf1c4b86d7b41 100644 (file)
--- a/client/src/com/vaadin/client/ui/VScrollTable.java
+++ b/client/src/com/vaadin/client/ui/VScrollTable.java
@@ -2774,7 +2774,9 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
 
         private boolean sortable = false;
         private final String cid;
+
         private boolean dragging;
+        private Integer currentDragX = null; // is used to resolve #14796
 
         private int dragStartX;
         private int colIndex;
@@ -3146,6 +3148,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                         event.stopPropagation();
                     }
                     dragging = true;
+                    currentDragX = WidgetUtil.getTouchOrMouseClientX(event);
                     moved = false;
                     colIndex = getColIndexByKey(cid);
                     DOM.setCapture(getElement());
@@ -3160,6 +3163,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                 if (columnReordering
                         && WidgetUtil.isTouchEventOrLeftMouseButton(event)) {
                     dragging = false;
+                    currentDragX = null;
                     DOM.releaseCapture(getElement());
 
                     if (WidgetUtil.isTouchEvent(event)) {
@@ -3227,47 +3231,57 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                 break;
             case Event.ONTOUCHMOVE:
             case Event.ONMOUSEMOVE:
-                if (dragging && WidgetUtil.isTouchEventOrLeftMouseButton(event)) {
-                    if (event.getTypeInt() == Event.ONTOUCHMOVE) {
-                        /*
-                         * prevent using this event in e.g. scrolling
-                         */
-                        event.stopPropagation();
-                    }
-                    if (!moved) {
-                        createFloatingCopy();
-                        moved = true;
-                    }
+                // only start the drag if the mouse / touch has moved a minimum
+                // distance in x-axis (the same idea as in #13381)
+                int currentX = WidgetUtil.getTouchOrMouseClientX(event);
 
-                    final int clientX = WidgetUtil
-                            .getTouchOrMouseClientX(event);
-                    final int x = clientX + tHead.hTableWrapper.getScrollLeft();
-                    int slotX = headerX;
-                    closestSlot = colIndex;
-                    int closestDistance = -1;
-                    int start = 0;
-                    if (showRowHeaders) {
-                        start++;
-                    }
-                    final int visibleCellCount = tHead.getVisibleCellCount();
-                    for (int i = start; i <= visibleCellCount; i++) {
-                        if (i > 0) {
-                            final String colKey = getColKeyByIndex(i - 1);
-                            // getColWidth only returns the internal width
-                            // without padding, not the offset width of the
-                            // whole td (#10890)
-                            slotX += getColWidth(colKey)
-                                    + scrollBody.getCellExtraWidth();
+                if (currentDragX == null
+                        || Math.abs(currentDragX - currentX) > VDragAndDropManager.MINIMUM_DISTANCE_TO_START_DRAG) {
+                    if (dragging
+                            && WidgetUtil.isTouchEventOrLeftMouseButton(event)) {
+                        if (event.getTypeInt() == Event.ONTOUCHMOVE) {
+                            /*
+                             * prevent using this event in e.g. scrolling
+                             */
+                            event.stopPropagation();
                         }
-                        final int dist = Math.abs(x - slotX);
-                        if (closestDistance == -1 || dist < closestDistance) {
-                            closestDistance = dist;
-                            closestSlot = i;
+                        if (!moved) {
+                            createFloatingCopy();
+                            moved = true;
                         }
-                    }
-                    tHead.focusSlot(closestSlot);
 
-                    updateFloatingCopysPosition(clientX, -1);
+                        final int clientX = WidgetUtil
+                                .getTouchOrMouseClientX(event);
+                        final int x = clientX
+                                + tHead.hTableWrapper.getScrollLeft();
+                        int slotX = headerX;
+                        closestSlot = colIndex;
+                        int closestDistance = -1;
+                        int start = 0;
+                        if (showRowHeaders) {
+                            start++;
+                        }
+                        final int visibleCellCount = tHead
+                                .getVisibleCellCount();
+                        for (int i = start; i <= visibleCellCount; i++) {
+                            if (i > 0) {
+                                final String colKey = getColKeyByIndex(i - 1);
+                                // getColWidth only returns the internal width
+                                // without padding, not the offset width of the
+                                // whole td (#10890)
+                                slotX += getColWidth(colKey)
+                                        + scrollBody.getCellExtraWidth();
+                            }
+                            final int dist = Math.abs(x - slotX);
+                            if (closestDistance == -1 || dist < closestDistance) {
+                                closestDistance = dist;
+                                closestSlot = i;
+                            }
+                        }
+                        tHead.focusSlot(closestSlot);
+
+                        updateFloatingCopysPosition(clientX, -1);
+                    }
                 }
                 break;
             default:

diff --git a/client/src/com/vaadin/client/ui/dd/VDragAndDropManager.java b/client/src/com/vaadin/client/ui/dd/VDragAndDropManager.java
index 844f4c1b9c09571e187691cfcbb6257c391cbd54..3a0b52b6afc92909b2cd2f5eb241b6cc5343ae2f 100644 (file)
--- a/client/src/com/vaadin/client/ui/dd/VDragAndDropManager.java
+++ b/client/src/com/vaadin/client/ui/dd/VDragAndDropManager.java
@@ -38,9 +38,9 @@ import com.vaadin.client.ComponentConnector;
 import com.vaadin.client.MouseEventDetailsBuilder;
 import com.vaadin.client.Profiler;
 import com.vaadin.client.UIDL;
-import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.VConsole;
 import com.vaadin.client.ValueMap;
+import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.ui.VOverlay;
 import com.vaadin.shared.ApplicationConstants;
 import com.vaadin.shared.MouseEventDetails;
@@ -240,6 +240,12 @@ public class VDragAndDropManager {
 
     }
 
+    /*
+     * #13381, #14796. The drag only actually starts when the mouse move or
+     * touch move event is more than 3 pixel away.
+     */
+    public static final int MINIMUM_DISTANCE_TO_START_DRAG = 3;
+
     private static VDragAndDropManager instance;
     private HandlerRegistration handlerRegistration;
     private VDragEvent currentDrag;
@@ -425,8 +431,8 @@ public class VDragAndDropManager {
                                 int currentY = WidgetUtil
                                         .getTouchOrMouseClientY(event
                                                 .getNativeEvent());
-                                if (Math.abs(startX - currentX) > 3
-                                        || Math.abs(startY - currentY) > 3) {
+                                if (Math.abs(startX - currentX) > MINIMUM_DISTANCE_TO_START_DRAG
+                                        || Math.abs(startY - currentY) > MINIMUM_DISTANCE_TO_START_DRAG) {
                                     if (deferredStartRegistration != null) {
                                         deferredStartRegistration
                                                 .removeHandler();

diff --git a/uitest/src/com/vaadin/tests/components/table/TableSortingStopsWorkingOnChrome.java b/uitest/src/com/vaadin/tests/components/table/TableSortingStopsWorkingOnChrome.java
new file mode 100644 (file)
index 0000000..23d0581
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/TableSortingStopsWorkingOnChrome.java
@@ -0,0 +1,150 @@
+package com.vaadin.tests.components.table;
+
+import java.io.Serializable;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.data.util.BeanItemContainer;
+import com.vaadin.event.dd.DragAndDropEvent;
+import com.vaadin.event.dd.DropHandler;
+import com.vaadin.event.dd.acceptcriteria.AcceptAll;
+import com.vaadin.event.dd.acceptcriteria.AcceptCriterion;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Table;
+import com.vaadin.ui.Table.ColumnReorderEvent;
+import com.vaadin.ui.Table.ColumnReorderListener;
+import com.vaadin.ui.Table.HeaderClickEvent;
+import com.vaadin.ui.Table.HeaderClickListener;
+import com.vaadin.ui.VerticalLayout;
+
+@Theme("valo")
+@SuppressWarnings("serial")
+public class TableSortingStopsWorkingOnChrome extends AbstractTestUI {
+
+    protected static final int ROW_COUNT = 100;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        VerticalLayout layout = new VerticalLayout();
+        layout.setSizeFull();
+
+        final Table table = new Table();
+        table.setColumnReorderingAllowed(true);
+        table.setSizeFull();
+
+        BeanItemContainer<TestItem> cont = new BeanItemContainer<TestItem>(
+                TestItem.class);
+
+        for (int i = 0; i < ROW_COUNT; i++) {
+            TestItem ti = new TestItem();
+            ti.setValue1("Value1_" + i);
+            ti.setValue2("Value2_" + (ROW_COUNT - i));
+            ti.setValue3("Value3_" + i);
+            ti.setValue4("Value4_" + (ROW_COUNT - i));
+            ti.setValue5("Value5_" + i);
+            cont.addBean(ti);
+        }
+
+        table.setContainerDataSource(cont);
+        table.setImmediate(true);
+        table.setSelectable(true);
+        table.setMultiSelect(false);
+
+        table.setPageLength(10);
+        table.setDragMode(Table.TableDragMode.ROW);
+
+        table.setDropHandler(new DropHandler() {
+            @Override
+            public void drop(DragAndDropEvent dragAndDropEvent) {
+
+            }
+
+            @Override
+            public AcceptCriterion getAcceptCriterion() {
+                return AcceptAll.get();
+            }
+        });
+
+        table.addColumnReorderListener(new ColumnReorderListener() {
+
+            @Override
+            public void columnReorder(ColumnReorderEvent event) {
+                System.out.println("columnReorder");
+            }
+        });
+
+        table.addHeaderClickListener(new HeaderClickListener() {
+
+            @Override
+            public void headerClick(HeaderClickEvent event) {
+                System.out.println("Header was clicked");
+            }
+        });
+
+        layout.addComponent(table);
+
+        addComponent(layout);
+    }
+
+    public class TestItem implements Serializable {
+        private static final long serialVersionUID = -745849615488792221L;
+
+        private String value1;
+        private String value2;
+        private String value3;
+        private String value4;
+        private String value5;
+
+        public String getValue1() {
+            return value1;
+        }
+
+        public void setValue1(String value1) {
+            this.value1 = value1;
+        }
+
+        public String getValue2() {
+            return value2;
+        }
+
+        public void setValue2(String value2) {
+            this.value2 = value2;
+        }
+
+        public String getValue3() {
+            return value3;
+        }
+
+        public void setValue3(String value3) {
+            this.value3 = value3;
+        }
+
+        public String getValue4() {
+            return value4;
+        }
+
+        public void setValue4(String value4) {
+            this.value4 = value4;
+        }
+
+        public String getValue5() {
+            return value5;
+        }
+
+        public void setValue5(String value5) {
+            this.value5 = value5;
+        }
+
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "After an indeterminate period of time sorting tables via clicking on the column header should not stop working on Chrome";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 14796;
+    }
+
+}