GrabDragInteraction issues when combining mouse/touch and alternative input

[jessegreenberg]

Initially discovered in phetsims/balloons-and-static-electricity#553 as a Firefox issue but then identified as a general problem when switching between mouse/touch and alternative input while using GrabDragInteraction in phetsims/balloons-and-static-electricity#553 (comment).

There are cases where the Node of the GrabDragInteraction can be released but still picked up by a DragListener. In phetsims/balloons-and-static-electricity#553 (comment) it was suggested that we don't call releaseDraggable in the blur listener if the GrabDragInteraction PressListener is pressed, but there are still cases where you can "release" the Node for the GrabDragInteraction but keep it dragged with a DragListener. I think the main problem is that the GrabDragInteraction is not aware of the the DragListener. The GrabDragInteraction has access to the KeyboardDragListener and interrupts it when switching from draggable to grabbable, I think we need to do the same with the DragListener. That means the GrabDragInteraction needs a reference to the DragListener.

[jessegreenberg]

Potential fix for this, where we can interrupt the DragListener:

Index: balloons-and-static-electricity/js/balloons-and-static-electricity/view/BalloonNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/balloons-and-static-electricity/js/balloons-and-static-electricity/view/BalloonNode.js b/balloons-and-static-electricity/js/balloons-and-static-electricity/view/BalloonNode.js
--- a/balloons-and-static-electricity/js/balloons-and-static-electricity/view/BalloonNode.js	(revision e47ad859441c5856a9551429b98fb873fb2032d5)
+++ b/balloons-and-static-electricity/js/balloons-and-static-electricity/view/BalloonNode.js	(date 1637348527960)
@@ -385,6 +385,9 @@
       objectToGrabString: accessibleLabelString,
       dragCueNode: interactionCueNode,
 
+
+      listenersToInterruptOnStateChange: [ dragHandler ],
+
       // BASE needs to control the ordering of all alerts after a release happens, so prevent
       // the default release alert
       // alertOnRelease: false,
Index: scenery-phet/js/accessibility/GrabDragInteraction.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery-phet/js/accessibility/GrabDragInteraction.js b/scenery-phet/js/accessibility/GrabDragInteraction.js
--- a/scenery-phet/js/accessibility/GrabDragInteraction.js	(revision 96c30a65f2a8a39b39c29398150f6aefc17b4572)
+++ b/scenery-phet/js/accessibility/GrabDragInteraction.js	(date 1637348479592)
@@ -111,6 +111,10 @@
         appendDescription: true // in general, the help text is after the grabbable
       },
 
+      // {Array<{interrupt:function():}>} - Any listeners with an interrupt function that should be interrupted when
+      // the state is changed between grabbable and draggable states.
+      listenersToInterruptOnStateChange: [],
+
       // {Object} - To pass in options to the cue. This is a scenery Node and you can pass it options supported by
       // that type. When positioning this node, it is in the target Node's parent coordinate frame.
       grabCueOptions: {},
@@ -285,6 +289,8 @@
     this.addAriaDescribedbyPredicate = options.addAriaDescribedbyPredicate;
     this.supportsGestureDescription = options.supportsGestureDescription;
 
+    this.listenersToInterruptOnStateChange = options.listenersToInterruptOnStateChange;
+
     // @private {number} - the number of times the component has been picked up for dragging, regardless
     // of pickup method for things like determining content for "hints" describing the interaction
     // to the user
@@ -663,6 +669,10 @@
     // input, but instead just those to be removed.
     listenersToRemove.forEach( listener => listener.interrupt && listener.interrupt() );
 
+    // Any additional listeners that should be interrupted when the state changes (such as a DragListener, so the
+    // KeyboardDragListener and other listeners to not fight in the draggable state)
+    this.listenersToInterruptOnStateChange.forEach( listener => listener.interrupt && listener.interrupt() );
+
     // remove all previous listeners from the node
     this.removeInputListeners( listenersToRemove );
 

Works well, but I wanted to do a bit more spot testing before committing.

[pixelzoom]

Reviewing scenery-phet issues for Q4 planning...

No progress in in 10 months. Assigning to @kathy-phet to prioritize.

[jessegreenberg]

I just confirmed that this is still an issue following the steps listed in phetsims/balloons-and-static-electricity#553 (comment).

[zepumph]

Subset of phetsims/scenery#1298