Fix partly missing drag image regression on Safari

Doesn't fix #9261, drag image missing on Safari when dragging grid row because
that has position: absolute and offset.

7 files changed, 190 insertions, 43 deletions

diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/GridDragSourceConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/GridDragSourceConnector.java
index 3f673bac01..134e76e7e5 100644
--- a/client/src/main/java/com/vaadin/client/connectors/grid/GridDragSourceConnector.java
+++ b/client/src/main/java/com/vaadin/client/connectors/grid/GridDragSourceConnector.java
@@ -27,6 +27,7 @@ import com.google.gwt.dom.client.Style;
 import com.google.gwt.dom.client.TableRowElement;
 import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.Window;
+import com.google.gwt.user.client.ui.Image;
 import com.vaadin.client.BrowserInfo;
 import com.vaadin.client.ServerConnector;
 import com.vaadin.client.WidgetUtil;
@@ -37,6 +38,7 @@ import com.vaadin.client.widgets.Escalator;
 import com.vaadin.client.widgets.Grid;
 import com.vaadin.shared.Range;
 import com.vaadin.shared.ui.Connect;
+import com.vaadin.shared.ui.dnd.DragSourceState;
 import com.vaadin.shared.ui.dnd.DropEffect;
 import com.vaadin.shared.ui.grid.GridDragSourceRpc;
 import com.vaadin.shared.ui.grid.GridDragSourceState;
@@ -98,33 +100,53 @@ public class GridDragSourceConnector extends DragSourceExtensionConnector {
             return;
         }
 
+        super.onDragStart(event);
+    }
+
+    @Override
+    protected void setDragImage(NativeEvent dragStartEvent) {
+        // do not call super since need to handle specifically
+        // 1. use resource if set (never needs safari hack)
+        // 2. add number badge if necessary (with safari hack if needed)
+        // 3. just use normal (with safari hack if needed)
+
         // Add badge showing the number of dragged columns
-        if (draggedItemKeys.size() > 1) {
-            Element draggedRowElement = (Element) event.getTarget();
-
-            Element badge = DOM.createSpan();
-            badge.setClassName(gridConnector.getWidget().getStylePrimaryName()
-                    + "-row" + STYLE_SUFFIX_DRAG_BADGE);
-            badge.setInnerHTML(draggedItemKeys.size() + "");
-
-            badge.getStyle().setMarginLeft(
-                    getRelativeX(draggedRowElement, (NativeEvent) event) + 10,
-                    Style.Unit.PX);
-            badge.getStyle().setMarginTop(
-                    getRelativeY(draggedRowElement, (NativeEvent) event)
-                            - draggedRowElement.getOffsetHeight() + 10,
-                    Style.Unit.PX);
-
-            draggedRowElement.appendChild(badge);
-
-            // Remove badge on the next animation frame. Drag image will still
-            // contain the badge.
-            AnimationScheduler.get().requestAnimationFrame(timestamp -> {
-                badge.removeFromParent();
-            }, (Element) event.getTarget());
+        String imageUrl = getResourceUrl(DragSourceState.RESOURCE_DRAG_IMAGE);
+        if (imageUrl != null && !imageUrl.isEmpty()) {
+            Image dragImage = new Image(
+                    getConnection().translateVaadinUri(imageUrl));
+            dragStartEvent.getDataTransfer()
+                    .setDragImage(dragImage.getElement(), 0, 0);
+        } else {
+            Element draggedRowElement = (Element) dragStartEvent
+                    .getEventTarget().cast();
+            if (draggedItemKeys.size() > 1) {
+
+                Element badge = DOM.createSpan();
+                badge.setClassName(
+                        gridConnector.getWidget().getStylePrimaryName() + "-row"
+                                + STYLE_SUFFIX_DRAG_BADGE);
+                badge.setInnerHTML(draggedItemKeys.size() + "");
+
+                badge.getStyle().setMarginLeft(
+                        getRelativeX(draggedRowElement, dragStartEvent) + 10,
+                        Style.Unit.PX);
+                badge.getStyle().setMarginTop(
+                        getRelativeY(draggedRowElement, dragStartEvent)
+                                - draggedRowElement.getOffsetHeight() + 10,
+                        Style.Unit.PX);
+
+                draggedRowElement.appendChild(badge);
+
+                // Remove badge on the next animation frame. Drag image will
+                // still contain the badge.
+                AnimationScheduler.get().requestAnimationFrame(timestamp -> {
+                    badge.removeFromParent();
+                }, (Element) dragStartEvent.getEventTarget().cast());
+                setDraggable(draggedRowElement);
+            }
+            fixDragImageForSafari(draggedRowElement);
         }
-
-        super.onDragStart(event);
     }
 
     private int getRelativeY(Element element, NativeEvent event) {
@@ -196,9 +218,9 @@ public class GridDragSourceConnector extends DragSourceExtensionConnector {
      * allowed and a selected row is dragged.
      *
      * @param draggedRow
-     *         Data of dragged row.
+     *            Data of dragged row.
      * @return {@code true} if multiple rows are dragged, {@code false}
-     * otherwise.
+     *         otherwise.
      */
     private boolean dragMultipleRows(JsonObject draggedRow) {
         SelectionModel<JsonObject> selectionModel = getGrid()
@@ -221,7 +243,7 @@ public class GridDragSourceConnector extends DragSourceExtensionConnector {
      * Get all selected rows from a subset of rows defined by {@code range}.
      *
      * @param range
-     *         Range of indexes.
+     *            Range of indexes.
      * @return List of data of all selected rows in the given range.
      */
     private List<JsonObject> getSelectedRowsInRange(Range range) {
@@ -241,7 +263,7 @@ public class GridDragSourceConnector extends DragSourceExtensionConnector {
      * Converts a list of {@link JsonObject}s to a {@link JsonArray}.
      *
      * @param objects
-     *         List of json objects.
+     *            List of json objects.
      * @return Json array containing all json objects.
      */
     private JsonArray toJsonArray(List<JsonObject> objects) {
@@ -257,7 +279,7 @@ public class GridDragSourceConnector extends DragSourceExtensionConnector {
      * otherwise.
      *
      * @param row
-     *         Row data.
+     *            Row data.
      * @return Drag data if present or row data otherwise.
      */
     private JsonObject getDragData(JsonObject row) {
diff --git a/client/src/main/java/com/vaadin/client/extensions/DragSourceExtensionConnector.java b/client/src/main/java/com/vaadin/client/extensions/DragSourceExtensionConnector.java
index 932128a8b3..e718fb90d7 100644
--- a/client/src/main/java/com/vaadin/client/extensions/DragSourceExtensionConnector.java
+++ b/client/src/main/java/com/vaadin/client/extensions/DragSourceExtensionConnector.java
@@ -15,9 +15,12 @@
  */
 package com.vaadin.client.extensions;
 
+import com.google.gwt.animation.client.AnimationScheduler;
 import com.google.gwt.dom.client.DataTransfer;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.NativeEvent;
+import com.google.gwt.dom.client.Style;
+import com.google.gwt.dom.client.Style.Position;
 import com.google.gwt.user.client.ui.Image;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.BrowserInfo;
@@ -163,7 +166,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
         }
 
         // Set drag image
-        setDragImage(event);
+        setDragImage(nativeEvent);
 
         // Set text data parameter
         String dataTransferText = createDataTransferText(event);
@@ -185,6 +188,48 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
     }
 
     /**
+     * Fixes missing drag image for Safari by making the dragged element
+     * position to relative if needed. Safari won't show drag image unless the
+     * dragged element position is relative or absolute / fixed, but not with
+     * display block for the latter.
+     * <p>
+     * This method is a NOOP for non-safari browser.
+     * <p>
+     * This fix is not needed if a custom drag image is used on Safari.
+     *
+     * @param draggedElement
+     *            the element that forms the drag image
+     */
+    protected void fixDragImageForSafari(Element draggedElement) {
+        if (!BrowserInfo.get().isSafari()) {
+            return;
+        }
+        final Style style = draggedElement.getStyle();
+        final String position = style.getPosition();
+
+        // relative works always
+        if ("relative".equalsIgnoreCase(position)) {
+            return;
+        }
+
+        // absolute & fixed don't work when there is offset used
+        if ("absolute".equalsIgnoreCase(position)
+                || "fixed".equalsIgnoreCase(position)) {
+            // FIXME #9261 need to figure out how to get absolute and fixed to
+            // position work when there is offset involved, like in Grid.
+            // The following hack with setting position to relative did not
+            // work, nor did clearing top/right/bottom/left.
+        }
+
+        // for all other positions, set the position to relative and revert it
+        // in an animation frame
+        draggedElement.getStyle().setPosition(Position.RELATIVE);
+        AnimationScheduler.get().requestAnimationFrame(timestamp -> {
+            draggedElement.getStyle().setProperty("position", position);
+        }, draggedElement);
+    }
+
+    /**
      * Creates data of type {@code "text"} for the {@code DataTransfer} object
      * of the given event.
      *
@@ -215,13 +260,16 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * @param dragStartEvent
      *            The drag start event.
      */
-    protected void setDragImage(Event dragStartEvent) {
+    protected void setDragImage(NativeEvent dragStartEvent) {
         String imageUrl = getResourceUrl(DragSourceState.RESOURCE_DRAG_IMAGE);
         if (imageUrl != null && !imageUrl.isEmpty()) {
             Image dragImage = new Image(
                     getConnection().translateVaadinUri(imageUrl));
-            ((NativeEvent) dragStartEvent).getDataTransfer()
+            dragStartEvent.getDataTransfer()
                     .setDragImage(dragImage.getElement(), 0, 0);
+        } else {
+            fixDragImageForSafari(
+                    (Element) dragStartEvent.getCurrentEventTarget().cast());
         }
     }
 
diff --git a/documentation/advanced/advanced-dragndrop.asciidoc b/documentation/advanced/advanced-dragndrop.asciidoc
index bcdfb2343d..57764b55dd 100644
--- a/documentation/advanced/advanced-dragndrop.asciidoc
+++ b/documentation/advanced/advanced-dragndrop.asciidoc
@@ -73,20 +73,24 @@ dragSource.addDragEndListener(event ->
 };
 ----
 
-[NOTE]
-====
-The browsers allow the user to select and drag and drop text, which may cause some issues when trying to drag a component that has text. You can fix this by setting the following style rule to the drag source component to prevent dragging of the text instead of the whole component.
-[source, css]
-----
-user-select: none;
-----
-====
-
 === CSS Style Rules
 
 The drag source element, additional to it's primary style name, have a style name with the `-dragsource` suffix. For example, a Label component would have the style name `v-label-dragsource` when the drag source extension is applied to it.
 Additionally, the elements also have the `v-draggable` style name that is independent of the component's primary style.
 
+The browsers allow the user to select and drag and drop text, which could cause issues with components that have text. The Framework tries to prevent this by automatically adding the following style to all `v-draggable` elements. It is included by the sass mixin `valo-drag-element`.
+
+[source, css]
+----
+.v-draggable {
+    -moz-user-select: none !important;
+    -ms-user-select: none !important;
+    -webkit-user-select: none !important;
+    user-select: none !important;
+}
+----
+
+
 [[advanced.dragndrop.drophandler]]
 == Drop Target
 
diff --git a/server/src/main/java/com/vaadin/event/dnd/ButtonDragSource.java b/server/src/main/java/com/vaadin/event/dnd/ButtonDragSource.java
index 4c681f5331..3d0d93dfa2 100644
--- a/server/src/main/java/com/vaadin/event/dnd/ButtonDragSource.java
+++ b/server/src/main/java/com/vaadin/event/dnd/ButtonDragSource.java
@@ -15,6 +15,7 @@
  */
 package com.vaadin.event.dnd;
 
+import com.vaadin.annotations.Widgetset;
 import com.vaadin.shared.ui.dnd.ButtonDragSourceState;
 import com.vaadin.ui.Button;
 
@@ -24,6 +25,7 @@ import com.vaadin.ui.Button;
  * @author Vaadin Ltd.
  * @since 8.1
  */
+@Widgetset("com.vaadin.DefaultWidgetSet")
 public class ButtonDragSource extends DragSourceExtension<Button> {
 
     public ButtonDragSource(Button target) {
diff --git a/themes/src/main/themes/VAADIN/themes/valo/shared/_overlay.scss b/themes/src/main/themes/VAADIN/themes/valo/shared/_overlay.scss
index 5b44e94cac..eabf4d3227 100644
--- a/themes/src/main/themes/VAADIN/themes/valo/shared/_overlay.scss
+++ b/themes/src/main/themes/VAADIN/themes/valo/shared/_overlay.scss
@@ -277,6 +277,9 @@ $v-selection-item-selection-color: $v-selection-color !default;
 
 @mixin valo-draggable {
   .v-draggable {
+    -moz-user-select: none !important;
+    -ms-user-select: none !important;
+    -webkit-user-select: none !important;
     user-select: none !important;
   }
 }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridDragAndDrop.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridDragAndDrop.java
index 149dec92e3..529d8cf562 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridDragAndDrop.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridDragAndDrop.java
@@ -122,7 +122,8 @@ public class GridDragAndDrop extends AbstractTestUIWithLog {
 
         // Add drag end listener
         dragSource.addGridDragEndListener(event -> {
-            if (event.getDropEffect() == DropEffect.MOVE) {
+            if (event.getDropEffect() == DropEffect.MOVE
+                    && draggedItems != null) {
                 // If drop is successful, remove dragged item from source Grid
                 ((ListDataProvider<Person>) grid.getDataProvider()).getItems()
                         .removeAll(draggedItems);
diff --git a/uitest/src/main/java/com/vaadin/tests/dnd/DragImage.java b/uitest/src/main/java/com/vaadin/tests/dnd/DragImage.java
new file mode 100644
index 0000000000..b4ef7ea035
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/dnd/DragImage.java
@@ -0,0 +1,67 @@
+package com.vaadin.tests.dnd;
+
+import java.util.stream.Stream;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.event.dnd.DragSourceExtension;
+import com.vaadin.server.Page;
+import com.vaadin.server.Page.Styles;
+import com.vaadin.server.ThemeResource;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.tests.components.uitest.TestSampler;
+import com.vaadin.ui.HorizontalLayout;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.VerticalLayout;
+
+@Theme("valo")
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class DragImage extends AbstractTestUIWithLog {
+
+    private final String[] positions = { "default", "relative", "absolute",
+            "static", "fixed", "sticky", "inherit", "initial", "unset" };
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        HorizontalLayout layout1 = new HorizontalLayout();
+        layout1.setCaption("No custom drag image");
+        Styles styles = Page.getCurrent().getStyles();
+
+        Stream.of(positions).forEach(position -> {
+            Label label = new Label(position);
+            label.addStyleName(position);
+            new DragSourceExtension<>(label);
+
+            layout1.addComponents(label, new Label("spacer"));
+
+            styles.add(new StringBuilder(".").append(position)
+                    .append(" { position: ").append(position).append(";}")
+                    .toString());
+        });
+
+        HorizontalLayout layout2 = new HorizontalLayout();
+        layout2.setCaption("Custom drag image");
+        Stream.of(positions).forEach(position -> {
+            Label label = new Label(position);
+            label.addStyleName(position);
+            new DragSourceExtension<>(label)
+                    .setDragImage(new ThemeResource(TestSampler.ICON_URL));
+            layout2.addComponents(label, new Label("spacer"));
+
+            styles.add(new StringBuilder(".").append(position)
+                    .append(" { position: ").append(position).append(";}")
+                    .toString());
+        });
+
+        // #9261 grid row like element that safari can't show the drag image for
+        String css = ".absolute-pos { position: absolute; top:0; }";
+        Label gridRowLikeLabel = new Label(css);
+        gridRowLikeLabel.addStyleName("absolute-pos");
+        new DragSourceExtension(gridRowLikeLabel);
+        styles.add(css);
+
+        addComponent(new VerticalLayout(layout1, layout2, gridRowLikeLabel));
+    }
+
+}