Fix HTML5 DnD regression for FF (#9245)

- Always set some drag data
- Set the dropEffect on dragEnter and dragOver events on drop target
- Send the dropEffect to server on drop event with disclaimer of current support
- Remove _dragOverCriteria_ and use _dropCriteria_ for `dragenter`, `dragover` and `drop` criteria

Tested manually basic DnD and Grid DnD on Mac with Chrome, Firefox, Safari.
Safari is still missing drag image (regression).

Tested manually basic DnD and Grid Dnd on Windows IE11 and Edge.
Drop event for both is still not working properly #9174.

13 files changed, 270 insertions, 210 deletions

diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/GridDropTargetConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/GridDropTargetConnector.java
index 09142e222b..8126f85c1e 100644
--- a/client/src/main/java/com/vaadin/client/connectors/grid/GridDropTargetConnector.java
+++ b/client/src/main/java/com/vaadin/client/connectors/grid/GridDropTargetConnector.java
@@ -82,7 +82,7 @@ public class GridDropTargetConnector extends DropTargetExtensionConnector {
 
     @Override
     protected void sendDropEventToServer(String dataTransferText,
-            Event dropEvent) {
+            String dropEffect, Event dropEvent) {
 
         String rowKey = null;
         DropLocation dropLocation = null;
@@ -96,8 +96,8 @@ public class GridDropTargetConnector extends DropTargetExtensionConnector {
                     (NativeEvent) dropEvent);
         }
 
-        getRpcProxy(GridDropTargetRpc.class).drop(dataTransferText, rowKey,
-                dropLocation);
+        getRpcProxy(GridDropTargetRpc.class).drop(dataTransferText, dropEffect,
+                rowKey, dropLocation);
     }
 
     private JsonObject getRowData(TableRowElement row) {
@@ -147,7 +147,7 @@ public class GridDropTargetConnector extends DropTargetExtensionConnector {
     }
 
     @Override
-    protected void setTargetIndicator(Event event) {
+    protected void setTargetClassIndicator(Event event) {
         getTargetRow(((Element) event.getTarget())).ifPresent(target -> {
 
             // Get required class name
@@ -184,7 +184,7 @@ public class GridDropTargetConnector extends DropTargetExtensionConnector {
     }
 
     @Override
-    protected void removeTargetIndicator(Event event) {
+    protected void removeTargetClassIndicator(Event event) {
 
         // Remove all possible style names
         getTargetRow((Element) event.getTarget()).ifPresent(e -> {
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 165e0ca9df..932128a8b3 100644
--- a/client/src/main/java/com/vaadin/client/extensions/DragSourceExtensionConnector.java
+++ b/client/src/main/java/com/vaadin/client/extensions/DragSourceExtensionConnector.java
@@ -77,7 +77,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * Sets the given element draggable and adds class name.
      *
      * @param element
-     *         Element to be set draggable.
+     *            Element to be set draggable.
      */
     protected void setDraggable(Element element) {
         element.setDraggable(Element.DRAGGABLE_TRUE);
@@ -91,7 +91,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * Removes draggable and class name from the given element.
      *
      * @param element
-     *         Element to remove draggable from.
+     *            Element to remove draggable from.
      */
     protected void removeDraggable(Element element) {
         element.setDraggable(Element.DRAGGABLE_FALSE);
@@ -104,7 +104,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * Adds dragstart and dragend event listeners to the given DOM element.
      *
      * @param element
-     *         DOM element to attach event listeners to.
+     *            DOM element to attach event listeners to.
      */
     protected void addDragListeners(Element element) {
         EventTarget target = element.cast();
@@ -117,7 +117,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * Removes dragstart and dragend event listeners from the given DOM element.
      *
      * @param element
-     *         DOM element to remove event listeners from.
+     *            DOM element to remove event listeners from.
      */
     protected void removeDragListeners(Element element) {
         EventTarget target = element.cast();
@@ -150,7 +150,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * dragstart} event occurs.
      *
      * @param event
-     *         browser event to be handled
+     *            browser event to be handled
      */
     protected void onDragStart(Event event) {
         // Convert elemental event to have access to dataTransfer
@@ -167,10 +167,12 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
 
         // Set text data parameter
         String dataTransferText = createDataTransferText(event);
-        if (dataTransferText != null && !dataTransferText.isEmpty()) {
-            nativeEvent.getDataTransfer()
-                    .setData(DragSourceState.DATA_TYPE_TEXT, dataTransferText);
+        // Always set something as the text data, or DnD won't work in FF !
+        if (dataTransferText == null) {
+            dataTransferText = "";
         }
+        nativeEvent.getDataTransfer().setData(DragSourceState.DATA_TYPE_TEXT,
+                dataTransferText);
 
         // Initiate firing server side dragstart event when there is a
         // DragStartListener attached on the server side
@@ -187,7 +189,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * of the given event.
      *
      * @param dragStartEvent
-     *         Event to set the data for.
+     *            Event to set the data for.
      * @return Textual data to be set for the event or {@literal null}.
      */
     protected String createDataTransferText(Event dragStartEvent) {
@@ -201,7 +203,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * handler attached.
      *
      * @param dragStartEvent
-     *         Client side dragstart event.
+     *            Client side dragstart event.
      */
     protected void sendDragStartEventToServer(Event dragStartEvent) {
         getRpcProxy(DragSourceRpc.class).dragStart();
@@ -211,7 +213,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * Sets the drag image to be displayed.
      *
      * @param dragStartEvent
-     *         The drag start event.
+     *            The drag start event.
      */
     protected void setDragImage(Event dragStartEvent) {
         String imageUrl = getResourceUrl(DragSourceState.RESOURCE_DRAG_IMAGE);
@@ -228,7 +230,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * event occurs.
      *
      * @param event
-     *         browser event to be handled
+     *            browser event to be handled
      */
     protected void onDragEnd(Event event) {
         // Initiate server start dragend event when there is a DragEndListener
@@ -248,9 +250,9 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * Initiates a server RPC for the drag end event.
      *
      * @param dragEndEvent
-     *         Client side dragend event.
+     *            Client side dragend event.
      * @param dropEffect
-     *         Drop effect of the dragend event, extracted from {@code
+     *            Drop effect of the dragend event, extracted from {@code
      *         DataTransfer.dropEffect} parameter.
      */
     protected void sendDragEndEventToServer(Event dragEndEvent,
@@ -269,11 +271,13 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
     }
 
     private native void setEffectAllowed(DataTransfer dataTransfer,
-            String effectAllowed)/*-{
+            String effectAllowed)
+    /*-{
         dataTransfer.effectAllowed = effectAllowed;
     }-*/;
 
-    private native String getDropEffect(DataTransfer dataTransfer)/*-{
+    static native String getDropEffect(DataTransfer dataTransfer)
+    /*-{
         return dataTransfer.dropEffect;
     }-*/;
 
@@ -282,7 +286,8 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
         return (DragSourceState) super.getState();
     }
 
-    private native boolean getStylePrimaryName(Element element)/*-{
+    private native boolean getStylePrimaryName(Element element)
+    /*-{
         return @com.google.gwt.user.client.ui.UIObject::getStylePrimaryName(Lcom/google/gwt/dom/client/Element;)(element);
     }-*/;
 }
diff --git a/client/src/main/java/com/vaadin/client/extensions/DropTargetExtensionConnector.java b/client/src/main/java/com/vaadin/client/extensions/DropTargetExtensionConnector.java
index e3fbeabab4..7c9e9755b8 100644
--- a/client/src/main/java/com/vaadin/client/extensions/DropTargetExtensionConnector.java
+++ b/client/src/main/java/com/vaadin/client/extensions/DropTargetExtensionConnector.java
@@ -25,6 +25,7 @@ import com.vaadin.client.ServerConnector;
 import com.vaadin.event.dnd.DropTargetExtension;
 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.dnd.DropTargetRpc;
 import com.vaadin.shared.ui.dnd.DropTargetState;
 
@@ -92,7 +93,7 @@ public class DropTargetExtensionConnector extends AbstractExtensionConnector {
      * DOM element.
      *
      * @param element
-     *         DOM element to attach event listeners to.
+     *            DOM element to attach event listeners to.
      */
     protected void addDropListeners(Element element) {
         EventTarget target = element.cast();
@@ -108,7 +109,7 @@ public class DropTargetExtensionConnector extends AbstractExtensionConnector {
      * given DOM element.
      *
      * @param element
-     *         DOM element to remove event listeners from.
+     *            DOM element to remove event listeners from.
      */
     protected void removeDropListeners(Element element) {
         EventTarget target = element.cast();
@@ -140,34 +141,68 @@ public class DropTargetExtensionConnector extends AbstractExtensionConnector {
      * Event handler for the {@code dragenter} event.
      *
      * @param event
-     *         browser event to be handled
+     *            browser event to be handled
      */
     protected void onDragEnter(Event event) {
-        // Generate style name for drop target
-        styleDragCenter = dropTargetWidget.getStylePrimaryName()
-                + STYLE_SUFFIX_DRAG_CENTER;
+        NativeEvent nativeEvent = (NativeEvent) event;
+        if (isDropAllowed(nativeEvent)) {
+            // Generate style name for drop target
+            styleDragCenter = dropTargetWidget.getStylePrimaryName()
+                    + STYLE_SUFFIX_DRAG_CENTER;
+
+            setTargetClassIndicator(event);
 
-        setTargetIndicator(event);
+            setDropEffect(nativeEvent);
+
+            // According to spec, need to call this for allowing dropping, the
+            // default action would be to reject as target
+            event.preventDefault();
+        } else {
+            // Remove drop effect
+            nativeEvent.getDataTransfer()
+                    .setDropEffect(DataTransfer.DropEffect.NONE);
+        }
+    }
+
+    /**
+     * Set the drop effect for the dragenter / dragover event, if one has been
+     * set from server side.
+     * <p>
+     * From Moz Foundation: "You can modify the dropEffect property during the
+     * dragenter or dragover events, if for example, a particular drop target
+     * only supports certain operations. You can modify the dropEffect property
+     * to override the user effect, and enforce a specific drop operation to
+     * occur. Note that this effect must be one listed within the effectAllowed
+     * property. Otherwise, it will be set to an alternate value that is
+     * allowed."
+     *
+     * @param event
+     *            the dragenter or dragover event.
+     */
+    protected void setDropEffect(NativeEvent event) {
+        if (getState().dropEffect != null) {
+
+            DataTransfer.DropEffect dropEffect = DataTransfer.DropEffect
+                    // the valueOf() needs to have equal string and name()
+                    // doesn't return in all upper case
+                    .valueOf(getState().dropEffect.name().toUpperCase());
+            event.getDataTransfer().setDropEffect(dropEffect);
+        }
     }
 
     /**
      * Event handler for the {@code dragover} event.
      *
      * @param event
-     *         browser event to be handled
+     *            browser event to be handled
      */
     protected void onDragOver(Event event) {
         NativeEvent nativeEvent = (NativeEvent) event;
-        if (isDragOverAllowed(nativeEvent)) {
-            // Set dropEffect parameter
-            if (getState().dropEffect != null) {
-                nativeEvent.getDataTransfer().setDropEffect(
-                        DataTransfer.DropEffect
-                                .valueOf(getState().dropEffect.name()));
-            }
+        if (isDropAllowed(nativeEvent)) {
+            setDropEffect(nativeEvent);
 
             // Add drop target indicator in case the element doesn't have one
-            setTargetIndicator(event);
+            setTargetClassIndicator(event);
 
             // Prevent default to allow drop
             nativeEvent.preventDefault();
@@ -178,60 +213,55 @@ public class DropTargetExtensionConnector extends AbstractExtensionConnector {
                     .setDropEffect(DataTransfer.DropEffect.NONE);
 
             // Remove drop target indicator
-            removeTargetIndicator(event);
-        }
-    }
-
-    /**
-     * Determines if dragover event is allowed on this drop target according to
-     * the dragover criteria.
-     *
-     * @param event
-     *         Native dragover event.
-     * @return {@code true} if dragover is allowed, {@code false} otherwise.
-     * @see DropTargetExtension#setDragOverCriteria(String)
-     */
-    protected boolean isDragOverAllowed(NativeEvent event) {
-        if (getState().dragOverCriteria != null) {
-            return executeScript(event, getState().dragOverCriteria);
+            removeTargetClassIndicator(event);
         }
-
-        // Allow when criteria not set
-        return true;
     }
 
     /**
      * Event handler for the {@code dragleave} event.
      *
      * @param event
-     *         browser event to be handled
+     *            browser event to be handled
      */
     protected void onDragLeave(Event event) {
-        removeTargetIndicator(event);
+        removeTargetClassIndicator(event);
     }
 
     /**
      * Event handler for the {@code drop} event.
      *
      * @param event
-     *         browser event to be handled
+     *            browser event to be handled
      */
     protected void onDrop(Event event) {
         NativeEvent nativeEvent = (NativeEvent) event;
-        if (dropAllowed(nativeEvent)) {
+        if (isDropAllowed(nativeEvent)) {
             nativeEvent.preventDefault();
             nativeEvent.stopPropagation();
 
-            String dataTransferText = nativeEvent.getDataTransfer().getData(
-                    DragSourceState.DATA_TYPE_TEXT);
+            String dataTransferText = nativeEvent.getDataTransfer()
+                    .getData(DragSourceState.DATA_TYPE_TEXT);
 
-            sendDropEventToServer(dataTransferText, event);
+            String dropEffect = DragSourceExtensionConnector
+                    .getDropEffect(nativeEvent.getDataTransfer());
+
+            sendDropEventToServer(dataTransferText, dropEffect, event);
         }
 
-        removeTargetIndicator(event);
+        removeTargetClassIndicator(event);
     }
 
-    private boolean dropAllowed(NativeEvent event) {
+    private boolean isDropAllowed(NativeEvent event) {
+        // there never should be a drop when effect has been set to none
+        if (getState().dropEffect != null
+                && getState().dropEffect == DropEffect.NONE) {
+            return false;
+        }
+        // TODO #9246: Should add verification for checking effectAllowed and
+        // dropEffect from event and comparing that to target's dropEffect.
+        // Currently Safari, Edge and IE don't follow the spec by allowing drop
+        // if those don't match
+
         if (getState().dropCriteria != null) {
             return executeScript(event, getState().dropCriteria);
         }
@@ -244,23 +274,26 @@ public class DropTargetExtensionConnector extends AbstractExtensionConnector {
      * Initiates a server RPC for the drop event.
      *
      * @param dataTransferText
-     *         Client side textual data that can be set for the drag source and
-     *         is transferred to the drop target.
+     *            Client side textual data that can be set for the drag source
+     *            and is transferred to the drop target.
+     * @param dropEffect
+     *            the desired drop effect
      * @param dropEvent
-     *         Client side drop event.
+     *            Client side drop event.
      */
     protected void sendDropEventToServer(String dataTransferText,
-            Event dropEvent) {
-        getRpcProxy(DropTargetRpc.class).drop(dataTransferText);
+            String dropEffect, Event dropEvent) {
+        getRpcProxy(DropTargetRpc.class).drop(dataTransferText, dropEffect);
     }
 
     /**
      * Add class that indicates that the component is a target.
      *
      * @param event
-     *         The drag enter or dragover event that triggered the indication.
+     *            The drag enter or dragover event that triggered the
+     *            indication.
      */
-    protected void setTargetIndicator(Event event) {
+    protected void setTargetClassIndicator(Event event) {
         getDropTargetElement().addClassName(styleDragCenter);
     }
 
@@ -270,13 +303,14 @@ public class DropTargetExtensionConnector extends AbstractExtensionConnector {
      * This is triggered on dragleave, drop and dragover events.
      *
      * @param event
-     *         the event that triggered the removal of the indicator
+     *            the event that triggered the removal of the indicator
      */
-    protected void removeTargetIndicator(Event event) {
+    protected void removeTargetClassIndicator(Event event) {
         getDropTargetElement().removeClassName(styleDragCenter);
     }
 
-    private native boolean executeScript(NativeEvent event, String script)/*-{
+    private native boolean executeScript(NativeEvent event, String script)
+    /*-{
         return new Function('event', script)(event);
     }-*/;
 
diff --git a/documentation/advanced/advanced-dragndrop.asciidoc b/documentation/advanced/advanced-dragndrop.asciidoc
index 804224d317..bcdfb2343d 100644
--- a/documentation/advanced/advanced-dragndrop.asciidoc
+++ b/documentation/advanced/advanced-dragndrop.asciidoc
@@ -130,17 +130,16 @@ When data is dragged over a drop target, the __v-drag-over__ class name is appli
 
 The __drop effect__ allows you to specify the desired drop effect, and for a succesful drop it must match the allowed effect that has been set for the drag source. Note that you can allow multiple effects, and that you should not rely on the default effect since it may vary between browsers.
 
-The __drag over criteria__ allows you determine whether the current drag source is allowed as a drop target, when the source is moved on top of the target. It is a script that is executed always when the `dragover` event is fired for the first time for this source, and returning `false` will prevent showing any drop effect. The script gets the `dragover` event as a parameter named `event`.
+The __drop criteria__ allows you to determine whether the current drag data can be dropped on the drop target. It is executed on `dragenter`, `dragover` and `drop` events. The script gets the current event as a parameter named `event`. Returning `false` will prevent the drop and no drop event is fired on the server side.
 
-The __drop criteria__ is similar to __drag over criteria__, but it is executed when the user has dropped the data by releasing the mouse button. The script gets the `drop` event as a parameter named `event`. Returning `false` will prevent the drop and no drop event is fired on the server side.
+////
+TODO Add an example of drop criteria
+////
 
 === CSS Style Rules
 
 When dragging data over a drop target and the drag over criteria passes, a style name is applied to indicate that the element accepts drops. This style name is the primary style name with `-drag-center` suffix, e.g. `v-label-drag-center`.
 
-////
-TODO Add an example of drag over criteria and drop criteria
-////
 
 ===
 
diff --git a/server/src/main/java/com/vaadin/event/dnd/DropEvent.java b/server/src/main/java/com/vaadin/event/dnd/DropEvent.java
index 932f5577a5..caa80a88ae 100644
--- a/server/src/main/java/com/vaadin/event/dnd/DropEvent.java
+++ b/server/src/main/java/com/vaadin/event/dnd/DropEvent.java
@@ -25,7 +25,7 @@ import com.vaadin.ui.Component;
  * Server side drop event. Fired when an HTML5 drop happens.
  *
  * @param <T>
- *         Type of the drop target component.
+ *            Type of the drop target component.
  * @author Vaadin Ltd
  * @see DropTargetExtension#addDropListener(DropListener)
  * @since 8.1
@@ -34,24 +34,28 @@ public class DropEvent<T extends AbstractComponent> extends Component.Event {
     private final String dataTransferText;
     private final DragSourceExtension<? extends AbstractComponent> dragSourceExtension;
     private final AbstractComponent dragSource;
+    private final DropEffect dropEffect;
 
     /**
      * Creates a server side drop event.
      *
      * @param target
-     *         Component that received the drop.
+     *            Component that received the drop.
      * @param dataTransferText
-     *         Data of type {@code "text"} from the {@code DataTransfer}
-     *         object.
+     *            Data of type {@code "text"} from the {@code DataTransfer}
+     *            object.
+     * @param dropEffect
+     *            the desired drop effect
      * @param dragSourceExtension
-     *         Drag source extension of the component that initiated the drop
-     *         event.
+     *            Drag source extension of the component that initiated the drop
+     *            event.
      */
-    public DropEvent(T target, String dataTransferText,
+    public DropEvent(T target, String dataTransferText, DropEffect dropEffect,
             DragSourceExtension<? extends AbstractComponent> dragSourceExtension) {
         super(target);
 
         this.dataTransferText = dataTransferText;
+        this.dropEffect = dropEffect;
 
         this.dragSourceExtension = dragSourceExtension;
         this.dragSource = Optional.ofNullable(dragSourceExtension)
@@ -70,9 +74,22 @@ public class DropEvent<T extends AbstractComponent> extends Component.Event {
     }
 
     /**
-     * Returns the drag source component if the drag originated from a
-     * component in the same UI as the drop target component, or an empty
-     * optional.
+     * Get the desired dropEffect for the drop event.
+     * <p>
+     * <em>NOTE: Currently you cannot trust this to work on all browsers!
+     * https://github.com/vaadin/framework/issues/9247 For Chrome & IE11 it is
+     * never set and always returns {@link DropEffect#NONE} even though the drop
+     * succeeded!</em>
+     *
+     * @return the drop effect
+     */
+    public DropEffect getDropEffect() {
+        return dropEffect;
+    }
+
+    /**
+     * Returns the drag source component if the drag originated from a component
+     * in the same UI as the drop target component, or an empty optional.
      *
      * @return Drag source component or an empty optional.
      */
@@ -81,9 +98,9 @@ public class DropEvent<T extends AbstractComponent> extends Component.Event {
     }
 
     /**
-     * Returns the extension of the drag source component if the drag
-     * originated from a component in the same UI as the drop target component,
-     * or an empty optional.
+     * Returns the extension of the drag source component if the drag originated
+     * from a component in the same UI as the drop target component, or an empty
+     * optional.
      *
      * @return Drag source extension or an empty optional
      */
@@ -97,7 +114,7 @@ public class DropEvent<T extends AbstractComponent> extends Component.Event {
      * drag source and drop target when they are in the same UI.
      *
      * @return Optional server side drag data if set and the drag source and the
-     * drop target are in the same UI, otherwise empty {@code Optional}.
+     *         drop target are in the same UI, otherwise empty {@code Optional}.
      * @see DragSourceExtension#setDragData(Object)
      */
     public Optional<Object> getDragData() {
diff --git a/server/src/main/java/com/vaadin/event/dnd/DropTargetExtension.java b/server/src/main/java/com/vaadin/event/dnd/DropTargetExtension.java
index a1b08980b2..53266a2eea 100644
--- a/server/src/main/java/com/vaadin/event/dnd/DropTargetExtension.java
+++ b/server/src/main/java/com/vaadin/event/dnd/DropTargetExtension.java
@@ -29,18 +29,18 @@ import com.vaadin.ui.AbstractComponent;
  * and drop.
  *
  * @param <T>
- *         Type of the component to be extended.
+ *            Type of the component to be extended.
  * @author Vaadin Ltd
  * @since 8.1
  */
-public class DropTargetExtension<T extends AbstractComponent> extends
-        AbstractExtension {
+public class DropTargetExtension<T extends AbstractComponent>
+        extends AbstractExtension {
 
     /**
      * Extends {@code target} component and makes it a drop target.
      *
      * @param target
-     *         Component to be extended.
+     *            Component to be extended.
      */
     public DropTargetExtension(T target) {
 
@@ -53,11 +53,12 @@ public class DropTargetExtension<T extends AbstractComponent> extends
      * Register server RPC.
      *
      * @param target
-     *         Extended component.
+     *            Extended component.
      */
     protected void registerDropTargetRpc(T target) {
-        registerRpc((DropTargetRpc) dataTransferText -> {
+        registerRpc((DropTargetRpc) (dataTransferText, dropEffect) -> {
             DropEvent<T> event = new DropEvent<>(target, dataTransferText,
+                    DropEffect.valueOf(dropEffect.toUpperCase()),
                     getUI().getActiveDragSource());
 
             fireEvent(event);
@@ -65,19 +66,28 @@ public class DropTargetExtension<T extends AbstractComponent> extends
     }
 
     /**
-     * Sets the drop effect for the current drop target. Used for the client
-     * side {@code DataTransfer.dropEffect} parameter.
+     * Sets the drop effect for the current drop target. This is set to the
+     * dropEffect on {@code dragenter} and {@code dragover} events.
+     * <p>
+     * <em>NOTE: If the drop effect that doesn't match the dropEffect /
+     * effectAllowed of the drag source, it DOES NOT prevent drop on IE and
+     * Safari! For FireFox and Chrome the drop is prevented if there they don't
+     * match.</em>
      * <p>
      * Default value is browser dependent and can depend on e.g. modifier keys.
+     * <p>
+     * From Moz Foundation: "You can modify the dropEffect property during the
+     * dragenter or dragover events, if for example, a particular drop target
+     * only supports certain operations. You can modify the dropEffect property
+     * to override the user effect, and enforce a specific drop operation to
+     * occur. Note that this effect must be one listed within the effectAllowed
+     * property. Otherwise, it will be set to an alternate value that is
+     * allowed."
      *
      * @param dropEffect
-     *         The drop effect to be set. Cannot be {@code null}.
+     *            the drop effect to be set or {@code null} to not modify
      */
     public void setDropEffect(DropEffect dropEffect) {
-        if (dropEffect == null) {
-            throw new IllegalArgumentException("Drop effect cannot be null.");
-        }
-
         if (!Objects.equals(getState(false).dropEffect, dropEffect)) {
             getState().dropEffect = dropEffect;
         }
@@ -86,78 +96,37 @@ public class DropTargetExtension<T extends AbstractComponent> extends
     /**
      * Returns the drop effect for the current drop target.
      *
-     * @return The drop effect of this drop target.
+     * @return The drop effect of this drop target or {@code null} if none set
+     * @see #setDropEffect(DropEffect)
      */
     public DropEffect getDropEffect() {
         return getState(false).dropEffect;
     }
 
     /**
-     * Sets criteria to allow dragover event on the current drop target. The
-     * script executes when dragover event happens and stops the event in case
-     * the script returns {@code false}.
+     * Sets criteria to allow drop on this drop target. The script executes when
+     * something is dragged on top of the target, and the drop is not allowed in
+     * case the script returns {@code false}.
      * <p>
      * <b>IMPORTANT:</b> Construct the criteria script carefully and do not
      * include untrusted sources such as user input. Always keep in mind that
      * the script is executed on the client as is.
      * <p>
      * Example:
-     * <pre>
-     *     target.setDropCriteria(
-     *         // If dragged source contains a URL, allow it to be dragged over
-     *         "if (event.dataTransfer.types.includes('text/uri-list')) {" +
-     *         "    return true;" +
-     *         "}" +
-     *
-     *         // Otherwise cancel the event"
-     *         "return false;");
-     * </pre>
-     *
-     * @param criteriaScript
-     *         JavaScript to be executed when dragover event happens or {@code
-     *         null} to clear.
-     */
-    public void setDragOverCriteria(String criteriaScript) {
-        if (!Objects.equals(getState(false).dragOverCriteria, criteriaScript)) {
-            getState().dragOverCriteria = criteriaScript;
-        }
-    }
-
-    /**
-     * Gets the criteria script that executes when dragover event happens. If
-     * the script returns {@code false}, the dragover event will be stopped.
      *
-     * @return JavaScript that executes when dragover event happens.
-     * @see #setDragOverCriteria(String)
-     */
-    public String getDragOverCriteria() {
-        return getState(false).dragOverCriteria;
-    }
-
-    /**
-     * Sets criteria to allow drop event on the current drop target. The script
-     * executes when drop event happens and stops the event in case the script
-     * returns {@code false}.
-     * <p>
-     * <b>IMPORTANT:</b> Construct the criteria script carefully and do not
-     * include untrusted sources such as user input. Always keep in mind that
-     * the script is executed on the client as is.
-     * <p>
-     * Example:
      * <pre>
-     *     target.setDropCriteria(
+     * target.setDropCriteria(
      *         // If dragged source contains a URL, allow it to be dropped
-     *         "if (event.dataTransfer.types.includes('text/uri-list')) {" +
-     *         "    return true;" +
-     *         "}" +
+     *         "if (event.dataTransfer.types.includes('text/uri-list')) {"
+     *                 + "    return true;" + "}" +
      *
-     *         // Otherwise cancel the event"
-     *         "return false;");
+     *                 // Otherwise cancel the event"
+     *                 "return false;");
      * </pre>
      *
      * @param criteriaScript
-     *         JavaScript to be executed when drop event happens or {@code null}
-     *         to clear.
+     *            JavaScript to be executed when drop event happens or
+     *            {@code null} to clear.
      */
     public void setDropCriteria(String criteriaScript) {
         if (!Objects.equals(getState(false).dropCriteria, criteriaScript)) {
@@ -166,8 +135,9 @@ public class DropTargetExtension<T extends AbstractComponent> extends
     }
 
     /**
-     * Gets the criteria script that executes when drop event happens. If the
-     * script returns {@code false}, the drop event will be stopped.
+     * Gets the criteria script that determines whether a drop is allowed. If
+     * the script returns {@code false}, then it is determined the drop is not
+     * allowed.
      *
      * @return JavaScript that executes when drop event happens.
      * @see #setDropCriteria(String)
@@ -177,12 +147,12 @@ public class DropTargetExtension<T extends AbstractComponent> extends
     }
 
     /**
-     * Attaches drop listener for the current drop target. {@link
-     * DropListener#drop(DropEvent)} is called when drop event happens on the
-     * client side.
+     * Attaches drop listener for the current drop target.
+     * {@link DropListener#drop(DropEvent)} is called when drop event happens on
+     * the client side.
      *
      * @param listener
-     *         Listener to handle drop event.
+     *            Listener to handle drop event.
      * @return Handle to be used to remove this listener.
      */
     public Registration addDropListener(DropListener<T> listener) {
diff --git a/server/src/main/java/com/vaadin/event/dnd/grid/GridDropEvent.java b/server/src/main/java/com/vaadin/event/dnd/grid/GridDropEvent.java
index 35421e7992..df634cf559 100644
--- a/server/src/main/java/com/vaadin/event/dnd/grid/GridDropEvent.java
+++ b/server/src/main/java/com/vaadin/event/dnd/grid/GridDropEvent.java
@@ -17,6 +17,7 @@ package com.vaadin.event.dnd.grid;
 
 import com.vaadin.event.dnd.DragSourceExtension;
 import com.vaadin.event.dnd.DropEvent;
+import com.vaadin.shared.ui.dnd.DropEffect;
 import com.vaadin.shared.ui.grid.DropLocation;
 import com.vaadin.ui.AbstractComponent;
 import com.vaadin.ui.Grid;
@@ -44,6 +45,8 @@ public class GridDropEvent<T> extends DropEvent<Grid<T>> {
      * @param dataTransferText
      *            Data of type {@code "text"} from the {@code DataTransfer}
      *            object.
+     * @param dropEffect
+     *            the desired drop effect
      * @param dragSourceExtension
      *            Drag source extension of the component that initiated the drop
      *            event.
@@ -53,9 +56,10 @@ public class GridDropEvent<T> extends DropEvent<Grid<T>> {
      *            Location of the drop within the target row.
      */
     public GridDropEvent(Grid<T> target, String dataTransferText,
+            DropEffect dropEffect,
             DragSourceExtension<? extends AbstractComponent> dragSourceExtension,
             T dropTargetRow, DropLocation dropLocation) {
-        super(target, dataTransferText, dragSourceExtension);
+        super(target, dataTransferText, dropEffect, dragSourceExtension);
 
         this.dropTargetRow = dropTargetRow;
         this.dropLocation = dropLocation;
diff --git a/server/src/main/java/com/vaadin/ui/GridDropTarget.java b/server/src/main/java/com/vaadin/ui/GridDropTarget.java
index 575e5b3f1c..021051130c 100644
--- a/server/src/main/java/com/vaadin/ui/GridDropTarget.java
+++ b/server/src/main/java/com/vaadin/ui/GridDropTarget.java
@@ -19,6 +19,7 @@ import com.vaadin.event.dnd.DropTargetExtension;
 import com.vaadin.event.dnd.grid.GridDropEvent;
 import com.vaadin.event.dnd.grid.GridDropListener;
 import com.vaadin.shared.Registration;
+import com.vaadin.shared.ui.dnd.DropEffect;
 import com.vaadin.shared.ui.grid.DropMode;
 import com.vaadin.shared.ui.grid.GridDropTargetRpc;
 import com.vaadin.shared.ui.grid.GridDropTargetState;
@@ -130,15 +131,16 @@ public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
 
     @Override
     protected void registerDropTargetRpc(Grid<T> target) {
-        registerRpc((GridDropTargetRpc) (dataTransferText, rowKey,
+        registerRpc((GridDropTargetRpc) (dataTransferText, dropEffect, rowKey,
                 dropLocation) -> {
 
             T dropTargetRow = target.getDataCommunicator().getKeyMapper()
                     .get(rowKey);
 
             GridDropEvent<T> event = new GridDropEvent<>(target,
-                    dataTransferText, getUI().getActiveDragSource(),
-                    dropTargetRow, dropLocation);
+                    dataTransferText,
+                    DropEffect.valueOf(dropEffect.toUpperCase()),
+                    getUI().getActiveDragSource(), dropTargetRow, dropLocation);
 
             fireEvent(event);
         });
diff --git a/shared/src/main/java/com/vaadin/shared/ui/dnd/DropTargetRpc.java b/shared/src/main/java/com/vaadin/shared/ui/dnd/DropTargetRpc.java
index 7a608068a1..a889fce098 100644
--- a/shared/src/main/java/com/vaadin/shared/ui/dnd/DropTargetRpc.java
+++ b/shared/src/main/java/com/vaadin/shared/ui/dnd/DropTargetRpc.java
@@ -30,8 +30,10 @@ public interface DropTargetRpc extends ServerRpc {
      * Called when drop event happens on client side.
      *
      * @param dataTransferText
-     *         Data of type {@code "text"} from the {@code DataTransfer}
-     *         object.
+     *            Data of type {@code "text"} from the {@code DataTransfer}
+     *            object.
+     * @param dropEffect
+     *            the desired drop effect
      */
-    public void drop(String dataTransferText);
+    public void drop(String dataTransferText, String dropEffect);
 }
diff --git a/shared/src/main/java/com/vaadin/shared/ui/dnd/DropTargetState.java b/shared/src/main/java/com/vaadin/shared/ui/dnd/DropTargetState.java
index 8605a78b25..c36d97f1dc 100644
--- a/shared/src/main/java/com/vaadin/shared/ui/dnd/DropTargetState.java
+++ b/shared/src/main/java/com/vaadin/shared/ui/dnd/DropTargetState.java
@@ -30,11 +30,6 @@ public class DropTargetState extends SharedState {
     public DropEffect dropEffect;
 
     /**
-     * Criteria script to allow dragOver event on the element
-     */
-    public String dragOverCriteria;
-
-    /**
      * Criteria script to allow drop event on the element
      */
     public String dropCriteria;
diff --git a/shared/src/main/java/com/vaadin/shared/ui/grid/GridDropTargetRpc.java b/shared/src/main/java/com/vaadin/shared/ui/grid/GridDropTargetRpc.java
index e0b51a0242..6fb4f39dd3 100644
--- a/shared/src/main/java/com/vaadin/shared/ui/grid/GridDropTargetRpc.java
+++ b/shared/src/main/java/com/vaadin/shared/ui/grid/GridDropTargetRpc.java
@@ -32,11 +32,13 @@ public interface GridDropTargetRpc extends ServerRpc {
      * @param dataTransferText
      *            Data of type {@code "text"} from the {@code DataTransfer}
      *            object.
+     * @param dropEffect
+     *            the desired drop effect
      * @param rowKey
      *            Key of the row on which the drop event occured.
      * @param dropLocation
      *            Location of the drop within the row.
      */
-    public void drop(String dataTransferText, String rowKey,
+    public void drop(String dataTransferText, String dropEffect, String rowKey,
             DropLocation dropLocation);
 }
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 3fde1c6bf4..149dec92e3 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
@@ -22,6 +22,8 @@ import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import com.vaadin.annotations.Theme;
+import com.vaadin.annotations.Widgetset;
 import com.vaadin.data.provider.ListDataProvider;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.shared.ui.dnd.DropEffect;
@@ -41,6 +43,8 @@ import com.vaadin.ui.RadioButtonGroup;
 import elemental.json.Json;
 import elemental.json.JsonObject;
 
+@Theme("valo")
+@Widgetset("com.vaadin.DefaultWidgetSet")
 public class GridDragAndDrop extends AbstractTestUIWithLog {
 
     private Set<Person> draggedItems;
@@ -61,13 +65,13 @@ public class GridDragAndDrop extends AbstractTestUIWithLog {
         grids.addComponents(left, right);
 
         // Selection modes
-        List<Grid.SelectionMode> selectionModes = Arrays.asList(
-                Grid.SelectionMode.SINGLE, Grid.SelectionMode.MULTI);
+        List<Grid.SelectionMode> selectionModes = Arrays
+                .asList(Grid.SelectionMode.SINGLE, Grid.SelectionMode.MULTI);
         RadioButtonGroup<Grid.SelectionMode> selectionModeSelect = new RadioButtonGroup<>(
                 "Selection mode", selectionModes);
         selectionModeSelect.setSelectedItem(Grid.SelectionMode.SINGLE);
-        selectionModeSelect.addValueChangeListener(event -> left
-                .setSelectionMode(event.getValue()));
+        selectionModeSelect.addValueChangeListener(
+                event -> left.setSelectionMode(event.getValue()));
 
         // Drop locations
         List<DropMode> dropLocations = Arrays.asList(DropMode.values());
@@ -113,9 +117,8 @@ public class GridDragAndDrop extends AbstractTestUIWithLog {
         });
 
         // Add drag start listener
-        dragSource.addGridDragStartListener(event ->
-                draggedItems = event.getDraggedItems()
-        );
+        dragSource.addGridDragStartListener(
+                event -> draggedItems = event.getDraggedItems());
 
         // Add drag end listener
         dragSource.addGridDragEndListener(event -> {
@@ -148,18 +151,17 @@ public class GridDragAndDrop extends AbstractTestUIWithLog {
                     List<Person> items = (List<Person>) dataProvider.getItems();
 
                     // Calculate the target row's index
-                    int index = items.indexOf(event.getDropTargetRow()) + (
-                            event.getDropLocation() == DropLocation.BELOW
-                                    ? 1 : 0);
+                    int index = items.indexOf(event.getDropTargetRow())
+                            + (event.getDropLocation() == DropLocation.BELOW ? 1
+                                    : 0);
 
                     // Add dragged items to the target Grid
                     items.addAll(index, draggedItems);
                     dataProvider.refreshAll();
 
-                    log("dragData=" + event.getDataTransferText()
-                            + ", target="
-                            + event.getDropTargetRow().getFirstName()
-                            + " " + event.getDropTargetRow().getLastName()
+                    log("dragData=" + event.getDataTransferText() + ", target="
+                            + event.getDropTargetRow().getFirstName() + " "
+                            + event.getDropTargetRow().getLastName()
                             + ", location=" + event.getDropLocation());
                 }
             });
diff --git a/uitest/src/main/java/com/vaadin/tests/dnd/DragAndDropCardShuffle.java b/uitest/src/main/java/com/vaadin/tests/dnd/DragAndDropCardShuffle.java
index 415dbf945d..8a136304aa 100644
--- a/uitest/src/main/java/com/vaadin/tests/dnd/DragAndDropCardShuffle.java
+++ b/uitest/src/main/java/com/vaadin/tests/dnd/DragAndDropCardShuffle.java
@@ -15,14 +15,24 @@
  */
 package com.vaadin.tests.dnd;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.annotations.Widgetset;
 import com.vaadin.event.dnd.DragSourceExtension;
 import com.vaadin.event.dnd.DropTargetExtension;
 import com.vaadin.server.Page;
 import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.dnd.DropEffect;
+import com.vaadin.shared.ui.dnd.EffectAllowed;
 import com.vaadin.tests.components.AbstractTestUIWithLog;
 import com.vaadin.ui.HorizontalLayout;
 import com.vaadin.ui.Label;
+import com.vaadin.ui.NativeSelect;
 
+@Theme("valo")
+@Widgetset("com.vaadin.DefaultWidgetSet")
 public class DragAndDropCardShuffle extends AbstractTestUIWithLog {
 
     // Create cards
@@ -34,8 +44,24 @@ public class DragAndDropCardShuffle extends AbstractTestUIWithLog {
     // Create desk
     private final HorizontalLayout desk = new HorizontalLayout();
 
+    private final List<DragSourceExtension<Label>> sources = new ArrayList<>();
+    private final List<DropTargetExtension<Label>> targets = new ArrayList<>();
+
     @Override
     protected void setup(VaadinRequest request) {
+        NativeSelect<EffectAllowed> effectAllowed = new NativeSelect<>(
+                "Effect Allowed (source)");
+        effectAllowed.setItems(EffectAllowed.values());
+        effectAllowed.setValue(EffectAllowed.UNINITIALIZED);
+        effectAllowed.setEmptySelectionAllowed(false);
+        effectAllowed.addValueChangeListener(event -> sources
+                .forEach(source -> source.setEffectAllowed(event.getValue())));
+
+        NativeSelect<DropEffect> dropEffect = new NativeSelect<>(
+                "Drop Effect (target)");
+        dropEffect.setItems(DropEffect.values());
+        dropEffect.addValueChangeListener(event -> targets
+                .forEach(target -> target.setDropEffect(event.getValue())));
 
         // Create UI and add extensions
         desk.addComponents(ace, jack, queen, king);
@@ -56,7 +82,7 @@ public class DragAndDropCardShuffle extends AbstractTestUIWithLog {
         addDragSourceExtension(king);
         addDropTargetExtension(king);
 
-        addComponent(desk);
+        addComponents(new HorizontalLayout(effectAllowed, dropEffect), desk);
 
         // Add styling
         setStyle();
@@ -70,7 +96,8 @@ public class DragAndDropCardShuffle extends AbstractTestUIWithLog {
         // Add listeners
         dragSource.addDragStartListener(event -> {
             event.getComponent().addStyleName("dragged");
-            log(event.getComponent().getValue() + " dragstart");
+            log(event.getComponent().getValue() + " dragstart, effectsAllowed="
+                    + event.getEffectAllowed());
         });
 
         dragSource.addDragEndListener(event -> {
@@ -78,6 +105,8 @@ public class DragAndDropCardShuffle extends AbstractTestUIWithLog {
             log(event.getComponent().getValue() + " dragend, dropEffect="
                     + event.getDropEffect());
         });
+
+        sources.add(dragSource);
     }
 
     private void addDropTargetExtension(Label target) {
@@ -94,28 +123,27 @@ public class DragAndDropCardShuffle extends AbstractTestUIWithLog {
                     // Swap source and target components
                     desk.replaceComponent(target, source);
 
-                    log(source.getValue() + " dropped onto " + event
-                            .getComponent().getValue());
+                    log(event.getComponent().getValue() + " drop received "
+                            + source.getValue() + ", dropEffect="
+                            + event.getDropEffect());
+                } else {
+                    log(event.getComponent().getValue()
+                            + " drop received something else than card");
                 }
             });
         });
+
+        targets.add(dropTarget);
     }
 
     private void setStyle() {
         Page.Styles styles = Page.getCurrent().getStyles();
 
-        styles.add(".card {"
-                + "width: 150px;"
-                + "height: 200px;"
-                + "border: 1px solid black;"
-                + "border-radius: 7px;"
-                + "padding-left: 10px;"
-                + "color: red;"
-                + "font-weight: bolder;"
-                + "font-size: 25px;"
-                + "background-color: gainsboro;"
-                + "}");
-        styles.add(".v-drag-over {border-style: dashed;}");
+        styles.add(".card {" + "width: 150px;" + "height: 200px;"
+                + "border: 1px solid black;" + "border-radius: 7px;"
+                + "padding-left: 10px;" + "color: red;" + "font-weight: bolder;"
+                + "font-size: 25px;" + "background-color: gainsboro;" + "}");
+        styles.add(".v-label-drag-center {border-style: dashed;}");
         styles.add(".dragged {opacity: .4;}");
     }