When screenReaderMode=true, keypress triggers unwanted default browser behaviors

[JasonXJ]

Details

• Browser and browser version: Chrome 109
• OS version: ChromeOS and others
• xterm.js version: ToT

Steps to reproduce

1. set screenReaderMode=true
2. Press keys such as ctrl+p / ctrl+f. Observe that the printing / find dialog pops up, steals the focus, and makes the user's life miserable 😢

This is caused by the code here, which cancels the key event only if screenReaderMode is off. I wonder if this is to allow the screen reader to handle some key bindings. I am not sure about other OSes, but on ChromeOS, screen reader bindings (e.g. Search+up) works fine even if the key event is canceled. I think we should just always cancel the key event. If there is indeed a need to pass through the key event, the user of xterm.js can set a custom key handler with attachCustomKeyEventHandler(). What do you think?

[JasonXJ]

So it appears that we don't cancel the keypress because we want it to insert the characters to the cursor <textarea> so that the screen reader will read it. The AccessibilityManager also has some code to avoid double announcing the characters (because the keypress will normally cause the character to be printed on the row, which triggers the AccessibilityManager). This logic is complicated and also half broken --- as soon as you use a key such as <backspace>, it will no longer be able to match _charsToAnnounce with the 0x7f in _charsToConsume, and AccessibilityManager will always double announce it.

I think the solution to all of these is to always cancel the keypress event, and always let AccessibilityManager announces what is being printed. What do you think? I will send over some code for review soon. I tested it, and it worked well for me.

[Tyriar]

Yes it's know that backspace will flush the textarea and cause problems, my plan is to eventually leverage shell integration in VS Code and provide hooks in xterm.js to set the textarea state (or just using Terminal.textarea).

I think the solution to all of these is to always cancel the keypress event, and always let AccessibilityManager announces what is being printed. What do you think? I will send over some code for review soon. I tested it, and it worked well for me.

I believe there's a difference between reading something out and reading out input, if memory serves me it was announced in a different tone. There is also the problem of then the input being dependent on the latency to the backend. So, I don't think this would work.

Should we check if ctrl is down and handle it differently here?

xterm.js/src/browser/Terminal.ts

Line 1063 in a351f57

if (!this.optionsService.rawOptions.screenReaderMode) {

[JasonXJ]

Hi, can you take a look at the new pull request and see if it is better?

Yes it's know that backspace will flush the textarea and cause problems, my plan is to eventually leverage shell integration in VS Code and provide hooks in xterm.js to set the textarea state (or just using Terminal.textarea).

The issue I am talking about is that keys (or key combinations) such as backspace (code '\x7F') is added to AccessibilityManager._charsToConsume, which will break the matching logic here and cause double announcement (once from the textarea, and once from the live region).

My last cl will fix this issue by announcing it only from the live region. Since you don't like it, I fix it differently in the new cl by not pusing to _charsToConsume if it contains any control character.

Should we check if ctrl is down and handle it differently here?

I think we should also check the alt key?

[JasonXJ]

thx!