Can't interact with Nodes with iOS VoiceOver when they are partially outside of the Display div

[jessegreenberg]

Related to #852 - Nodes are not draggable on iOS Safari with VoiceOver if the center of the node in the global coordinate frame is out of bounds. This was discovered in phetsims/gravity-force-lab#210.

@zepumph and @jessegreenberg discussed this and decided that the best way forward is to explore modifying the AccessiblePeer sibling bounds to only contain the bounds that are visible and fully contained by the display div. This way the "fake" pointer events that are provided by the screen reader will always be within display div bounds.

This is high priority because it is required before publication of gravity-force-lab-basics.

[jessegreenberg]

I am not able to test on an iPad at the moment but this patch is working well to constrain the bounds to display bounds

diff --git a/js/accessibility/AccessiblePeer.js b/js/accessibility/AccessiblePeer.js
index 09d70706..2f572d7d 100644
--- a/js/accessibility/AccessiblePeer.js
+++ b/js/accessibility/AccessiblePeer.js
@@ -888,25 +888,33 @@ define( require => {
         if ( scratchGlobalBounds.isFinite() ) {
           scratchGlobalBounds.transform( this.accessibleInstance.transformTracker.getMatrix() );
 
-          let clientDimensions = getClientDimensions( this._primarySibling );
-          let clientWidth = clientDimensions.width;
-          let clientHeight = clientDimensions.height;
-
-          if ( clientWidth > 0 && clientHeight > 0 ) {
-            scratchSiblingBounds.setMinMax( 0, 0, clientWidth, clientHeight );
-            scratchSiblingBounds.transform( getCSSMatrix( clientWidth, clientHeight, scratchGlobalBounds ) );
-            setClientBounds( this._primarySibling, scratchSiblingBounds );
-          }
+          const displayBounds = this.display.bounds;
+          if ( displayBounds.intersectsBounds( scratchGlobalBounds ) ) {
+
+            // constrain the bounds to what is contained within the Display so that VoiceOver sends an event to what
+            // is visible
+            scratchGlobalBounds.constrainBounds( this.display.bounds );
 
-          if ( this.labelSibling ) {
-            clientDimensions = getClientDimensions( this._labelSibling );
-            clientWidth = clientDimensions.width;
-            clientHeight = clientDimensions.height;
+            let clientDimensions = getClientDimensions( this._primarySibling );
+            let clientWidth = clientDimensions.width;
+            let clientHeight = clientDimensions.height;
 
-            if ( clientHeight > 0 && clientWidth > 0 ) {
+            if ( clientWidth > 0 && clientHeight > 0 ) {
               scratchSiblingBounds.setMinMax( 0, 0, clientWidth, clientHeight );
               scratchSiblingBounds.transform( getCSSMatrix( clientWidth, clientHeight, scratchGlobalBounds ) );
-              setClientBounds( this._labelSibling, scratchSiblingBounds );
+              setClientBounds( this._primarySibling, scratchSiblingBounds );
+            }
+
+            if ( this.labelSibling ) {
+              clientDimensions = getClientDimensions( this._labelSibling );
+              clientWidth = clientDimensions.width;
+              clientHeight = clientDimensions.height;
+
+              if ( clientHeight > 0 && clientWidth > 0 ) {
+                scratchSiblingBounds.setMinMax( 0, 0, clientWidth, clientHeight );
+                scratchSiblingBounds.transform( getCSSMatrix( clientWidth, clientHeight, scratchGlobalBounds ) );
+                setClientBounds( this._labelSibling, scratchSiblingBounds );
+              }
             }
           }
         }

[jessegreenberg]

This is fixed with the above commit and I tested on my iPad Air 2. @zepumph can you please verify?

[zepumph]

Yes I verified that this is working. It is working very well for GFL.

I still have potential weariness about this in the general case. Because of the "snap to center" bug from phetsims/gravity-force-lab#191, we can see first hand that the center is being calculated from the "on screen" bounds. With this change there is now much more possibility of dragging bugginess where the item is off screen such that the "center" of the element (really just the center of the bounds) is not actually on the element, and as a result this will break dragging of that object. This is of course a very niche bug in which all these must be true:

• gesture a11y (right now meaning on iOS with VO)
• application role, since that is the only role that we care about these pointer events
• a draggable object that can move off screen at least partially
• an irregular shape that is not draggable in a portion of its on screen center when off screen.

I wanted to point out the limitation while also not recommending any further steps. Back to you. Anything else for this issue?

[jessegreenberg]

That is correct. This algorithm assumes that the entire area of the target Node/transform source node is pickable. I think that is acceptable for now. Thanks for verify/review. Closing this issue.