Check Focus when exiting Keyboard Shortcuts Dialog

[terracoda]

In an interview with a JAWS user, the focus after closing the Keyboard Shortcuts dialog seemed incorrect.

AT: JAWS, version 18 Professional
Browser: Firefox, version 52

User closed the dialog, and focus went back to the top of the sim or to the beginning of the Play Area, not sure which.

The expected behaviour would be to go to the Keyboard Shortcuts dialog button.

On re-testing with VO, I see if I read through part of the dialog and then hit escape, visual keyboard focus is lost and the virtual cursor seems to be at the beginning of Sim Resources region.

Could we make sure that upon closing the dialog with any method (ESC or Close button) that the visual focus is placed on the Keyboard Shortcuts button.

[jessegreenberg]

Thanks @terracoda, I think QA found this too, is this the same as #141?

[jessegreenberg]

Hmm, #141 was for Edge, this is for Firefox.

[jessegreenberg]

Focus management is correct for Chrome.

[jessegreenberg]

Focus management is correct in firefox. Why does using a screen reader cause a problem here?

[jessegreenberg]

Focus is lost when using Firefox and NVDA.

[jessegreenberg]

#141 is fixed, but it didn't cover this issue. I am curious why this hasn't been a problem in previous releases.

[jessegreenberg]

The implemenation of updateVisibility is presumably the biggest difference in AccessibleInstance.

[jessegreenberg]

If I remove this line in AccessibleInstance:

      parentElement.hidden = !( visibilityCount === self.trailDiff.length );

the problem is still there, so it is likely caused by something else.

[jessegreenberg]

Printing what Dialog is trying to restore focus to after closing, I see a reference to the help button, so focus may be lost after we are focusing the button.

[jessegreenberg]

I am quite stumped. The bug is also not happening consistently, about 20% of the time, NVDA put focus on the help button.

[terracoda]

@jessegreenberg, it seems the management of focus is not always trivial. What are the tools/methods you use to manage focus?

In the case of a pop-up dialog, focus in placed in the dialog (kept there until the dialog closes). Upon closing, how is focus managed at that point?

[jessegreenberg]

Upon closing, how is focus managed at that point?

I am using JavaScript to send focus back to the KeyboardHelpButton. #143 (comment) and the fact that this bug is not there when AT are not in use indicate that the JavaScript is sound.

[terracoda]

Very strange, that the focus management is affected by the AT being used.

[jessegreenberg]

Ok... So I added this to KeyboardHelpDialog to try to figure out what is going on.

        click: function() {
          console.log( 'click' );
          debugger;
          self.hide();
          self.focusActiveElement();
        }

When NVDA is off, I see "click" in the console and hit the debugger. When I turn NVDA on, I no longer see "click" and I no longer hit the debugger?! Is this an aggressive caching issue?

[terracoda]

I don't know what it is, sorry.

[jessegreenberg]

@zepumph helped me with this today, we got very inconsistent results while testing. While the dev tools were open, we didn't observe the bug. While the dev tools are closed, this is happening relatively consistently with NVDA. We discovered that the BarrierRectangle fire listener is somehow being called. We were able to catch the stack trace after closing the KeyboardHelpDialog with 'enter':

18:18:32.528 Error: dangit NVDA 1 BarrierRectangle.js:44:15
  BarrierRectangle/<.fire http://localhost:8080/scenery-phet/js/BarrierRectangle.js:44:15
  .setButtonState http://localhost:8080/scenery/js/input/ButtonListener.js:114:11
  ButtonListener/<.up http://localhost:8080/scenery/js/input/ButtonListener.js:75:9
  .buttonUp http://localhost:8080/scenery/js/input/DownUpListener.js:129:9
  DownUpListener/this.downListener.up http://localhost:8080/scenery/js/input/DownUpListener.js:62:11
  .dispatchToListeners http://localhost:8080/scenery/js/input/Input.js:901:56
  .dispatchEvent http://localhost:8080/scenery/js/input/Input.js:871:7
  .upEvent http://localhost:8080/scenery/js/input/Input.js:720:7
  .touchEnd http://localhost:8080/scenery/js/input/Input.js:481:9
  .run http://localhost:8080/scenery/js/input/BatchedDOMEvent.js:67:11
  .fireBatchedEvents http://localhost:8080/scenery/js/input/Input.js:218:11
  .batchEvent http://localhost:8080/scenery/js/input/Input.js:194:11
  BrowserEvents.batchWindowEvent http://localhost:8080/scenery/js/input/BrowserEvents.js:279:9
  ontouchend http://localhost:8080/scenery/js/input/BrowserEvents.js:502:7
  EventLoop.prototype.enter resource://devtools/server/actors/script.js:349:5
  bound  self-hosted:913:17
  ThreadActor<._pushThreadPause resource://devtools/server/actors/script.js:532:5
  ThreadActor<._pauseAndRespond resource://devtools/server/actors/script.js:740:7
  ThreadActor<._makeSteppingHooks/steppingHookState.pauseAndRespond resource://devtools/server/actors/script.js:869:16
  ThreadActor<._makeOnEnterFrame/< resource://devtools/server/actors/script.js:760:11
  .hide http://localhost:8080/sun/js/Dialog.js:286:1
  KeyboardHelpDialog/clickListener<.click http://localhost:8080/joist/js/KeyboardHelpDialog.js:130:11
  Accessibility.compose/<.addAccessibleInputListener/addedAccessibleInput[ev] http://localhost:8080/scenery/js/accessibility/Accessibility.js:443:19

This led us to believe that perhaps NVDA is sending fake touch events the the browser. When we observe the bug with ?sceneryLog=Input, we see this.

The "Barrier Rectangle input listener" output was logged at the same time of the bug in each case above. This line indicates where the bug occurs (but not the bug itself).

[zepumph]

@emily-phet said that this is not necessary to fix before the next RIAW RC. Proceeding with the cherry picking and QA process.

[jessegreenberg]

The above commit fixes this problem by abstaining from blurring if the AT submitted a down event at the location of the PDOM outside of the display. VO was sometimes doing something similar in phetsims/balloons-and-static-electricity#401. I tested with JAWS in FF Quantum to verify. Closing.