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.

4 files changed, 83 insertions, 90 deletions

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);
         });