Some AccessiblePeer siblings are not clickable on iOS13

[jessegreenberg]

From phetsims/gravity-force-lab-basics#176

At least for input type=checkbox. It appears that width of the checkbox cannot be set with element.style.width:

In Chrome, we can see that the width is set correctly and takes the width of the entire Checkbox node with label:

This needs to be fixed and we should test other elements for the problem.

[jessegreenberg]

The VoiceOver highlight surrounds the entire checkbox with label when focused, which indicates that width is being set correctly and somewhat respected. Its interesting that the size of the actual box matches ours, but I think that is a coincidence.

[jessegreenberg]

In Safari, clicking to the right of the checkbox still clicks the box. So this should be totally fine. So this probably isn't causing the problem and isn't what we need to fix.

[jessegreenberg]

The orange shows the dimensions of the HTML checkbox in Safari:

But the interactive box isn't centered in this width.

[jessegreenberg]

In a separate HTML context, I saw that checkboxes cannot be clicked with pointer-events: none but the label could still be clicked. For this reason I thought that maybe pointer-events: none was preventing the checkbox from being clicked. But I confirmed that we are still receiving touch events and hitting the touchStartAction of Input.js. So scenery should be able to receive and handle. Maybe the pointer event isn't at the correct spot?

[jessegreenberg]

Ah, when I click a checkbox outside of the sim I hear a single beep from the screen reader. In the sim I hear two very rapidly back to back, so I think it is getting checked/unchecked right in a row.

In this case scenery is only receiving a single touch event.

[jessegreenberg]

Here are the events that we receive from a single click of a checkbox on this platform. This is very surprising:

So it looks like the checkbox is getting pressed, then released, then focused, then clicked, then receives input and change events. This explains why it is getting clicked and un-clicked immediately.

[terracoda]

@jessegreenberg, nice detective work.
I hope you can find a way to real in all those events! It is not clear to me if this is an iOS bug or a scenery bug.

[jessegreenberg]

So it looks like the root of the problem is that iOS no longer respects pointer-events: none to prevent interaction with VoiceOver. I think the reason is because VO on iOS 12 sent fake pointerdown/pointerup events to the target. But now VoiceOver sends fake mousedown/mouseup events to the targets.

As an example, this HTML button cannot be clicked with a finger on iOS 13, and cannot be clicked by a mouse in windows chrome.

<button id='native-button' style="pointer-events: none;">TEST</button>

But when VO is enabled, the button can be clicked by double tapping the screen on iOS 13.

I would be surprised if this is intentional and will send a bug report to apple.

pointer-events: none none was previously used to prevent the PDOM HTML from receiving events so that the Display div can capture everything.

[jessegreenberg]

A very similar issue came up in #852, and I wonder if a modification of that can be applied to resolve this.

[jessegreenberg]

The best solution that I could come up with tonight is to "block" dispatch of a click event when an element in the PDOM receives a mouse/pointer event. the PDOM should only receive mouse/pointer events when given fake ones from a screen reader. This may be OK, but feels a bit fragile. A quick trial shows it working OK.

[jessegreenberg]

I am finding problems with #1001 (comment), buttons don't "look" pressed when using NVDA + Firefox. Maybe because the down/up is so fast? And buttons are not clickable with NVDA + Chrome. It looks like fake pointer events are now being received on that platform too.

But I just discovered that the existence of a mousedown listener changes the events that an element receives with iOS 13 VoiceOver. If a mouseover event listener is on a button, the element will receive
enter, over, blur, down, up, out, exit, focus, click, input, and change events. When the mousdown event listener is removed, we only receive focus, input, click, and change events. This is so confusing.

So removing the woraround for phetsims/a11y-research#133 makes things work OK in iOS 13 Safari + VO.

[terracoda]

I agree this is super confusing!

[jessegreenberg]

@zepumph paired on this with me yesterday and helped out quite a lot. During our discussion, we identified a few questions that Ill try to answer here:

In Safari, the pointer event reaches the display div before it reaches the PDOM. Why? This made it impossible to capture the event and stop propagation when the event target was an element in the PDOM.

I am not certain but at this time I am guessing it has something to do with how Safari handles events with position: fixed. The sim is on top of the html elements in the z index so maybe layer is prefered in this case?

It seems like mobile VO is sending other input/change/click events to PDOM elements now. Do we still need to transform AccessiblePeer siblings to capture pointer events?

Yes we do, one of the features that comes out of positioning is being able to tap on various sim elements on the screen to hear description content. Without positioning this would not be possible.

Yesterday, we were looking at using stopPropagation to prevent pointer listeners from being called in this case. This morning I tried a different way that just prevents the Scenery down event from dispatching if the DOMEvent target is within the PDOM. This seems to be working well. I verified that this will still work with drag and drop because for that gesture, the event target is the Display div and not the PDOM element. Going to test just a bit more.

[jessegreenberg]

This is working well, I tested that everything is still clickable with NVDA, JAWS, and mobile VO. @KatieWoe could you please also verify that this is fixed on iOS 13 by testing gravity-force-lab-basics master with VoiceOver?

[KatieWoe]

Looks ok on master on iPadOS 13

[zepumph]

@jessegreenberg, the phet-io dev team is getting an issue in FAMB that I think has to do with the above commit. This happens when I click anywhere in the sim in the state wrapper (untested elsewhere). Consistently reproducible with http://localhost/phet-io-wrappers/state/?sim=forces-and-motion-basics&phetioValidateTandems&phetioDebug&screens=2&phetioValidateAPI=false

Uncaught TypeError: Cannot read property 'peer' of undefined
    at Display.getAccessibleDOMElement (Display.js:688)
    at Display.get accessibleDOMElement [as accessibleDOMElement] (Display.js:690)
    at Input.downEvent (Input.js:1562)
    at Input.mouseDownAction.Action.phetioPlayback (Input.js:295)
    at Action.execute (Action.js:238)
    at Input.mouseDown (Input.js:1048)
    at Input.pointerDown (Input.js:1293)
    at BatchedDOMEvent.run (BatchedDOMEvent.js:58)
    at Input.fireBatchedEvents (Input.js:807)
    at Input.batchEvent (Input.js:768)

[jessegreenberg]

Thanks @zepumph, very sorry about that, above commit should fix this (verified with aqua).

[KatieWoe]

Ok, still looks fixed. I did have trouble moving mass up, it only seemed to want to move down. Don't know if it is connected to this fix.

[jessegreenberg]

Thanks for checking @KatieWoe, was able to reproduce that. A version of GFLB on 1.0 shas with this change does not have this problem so I suspect it is caused by something else in master recently.

[jessegreenberg]

Alright, this is fixed now. @zepumph would you mind reviewing before merging changes into GFLB release branch?

[zepumph]

I was a bit suspicious about how few usages there were of this, but then after investigation I see that upEvent is down stream of many pointer types. The fix looks good. I'll add it to an RC and see how it goes.

[zepumph]

I will commit the following commits to the branch over in phetsims/gravity-force-lab-basics#179, in order:
92b6451
b2fe5e8
1c4830c

[zepumph]

I will add commits here, but will flag phetsims/gravity-force-lab-basics#176 for QA testing. Removing priority, and I will update this issue as needed. Thanks @jessegreenberg.

[jessegreenberg]

Looks like this was verified as fixed in master and the RC, this can be closed.