Improves input help for keyboard

[Emil-18]

Link to issue number:

fixes #15891, #6621

Summary of the issue:

When the key combination isn't a script, input help does not report the resulting character when holding down modifier keys and press a character key, e.g instead of reporting bang (!)in the english keyboard layout, it reports shift+1. There is also no way to know quickly if a key combination is a NVDA command or not, e.g the user hav to potentialy listen to modifier name+modifier name+ modifier name+main key name, only to find out that the key combination isn't a NVDA command

Description of user facing changes

When input help is on, if the key combination is a NVDA command, NVDA reports it as it always have. If not, but the key combination would have resulted in a typed character if input help was off, this character is reported. Else the modifier keys and the main key is reported as before, just with the + sign replaced with space, so the user can more quickly know that the key combination isn't a NVDA command

Description of development approach

When the gesture don't have a script, NVDA will get the character corresponding to the pressed keys. It does this by creating a keyState list, and set the values corresponding to the modifiers that are down according to NVDA to -128, wich will tell ToUnicodeEx that the key is down. It then sens this list to ToUnicodeEx, and also gives it the keyboard layout for the focused window. If ToUnicodeEx returns a negative value, wich means that a dead key character was pressed, it returns, as I personally think that had it reported the key, it may misleed users into thinking they would have written that character when pressing the key. It then checks if alt is pressed, as when alt is pressed, ToUnicodeEx sometimes gives the same character as if alt was not pressed. If it is pressed, alt is then removed from the key states and ToUnicodeEx is called again. If the two calls gives the same characters, they are counted as invalid. They are also counted as invalid if they are not printable, are space, or if the windows key is pressed. If ToUnicodeEx gave characters and all the characters are valid, these are reported. If not, + are replaced with space in textList and reported as normal.

Testing strategy:

I have tested with both the english and norwegian keyboard layouts, on a laptop keyboard without numbpad. Everything works as expected, except for the issue described below

Known issues with pull request:

If typing a dead key character in input help, and input help is exited without typing another character, the next typed character will be typed with the ded key character

Code Review Checklist:

• [x] Documentation:
  • Change log entry
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• [x] Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• [x] UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English
• [x] API is compatible with existing add-ons.
• [x] Security precautions taken.

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests (tags: installer NVDA).
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/wxh3dqk6svpa1g75/artifacts/output/nvda_snapshot_pr15907-30295,a037005b.exe
• CI timing (mins): INIT 0.0, INSTALL_START 0.9, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 28.0, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 17.0, FINISH_END 0.1

See test results for failed build of commit a037005b74

[CyrilleB79]

Personally, I was not a big fan of this change, but it seems that it was expected by many people; so that's probably the way to go.

Testing this PR, this seems to work quite well, except the small issue with the pending dead key when exiting input help mode as already described in known issues; if no solution is found, this issue is probably acceptable though.

You have done the choice not to report dead keys when pressed in input help mode, and to report the modified letter on next press instead. Personally, I would have preferred the other solution, i.e. report dead key immediately and do not modify the next character. But that's just a matter of preference and I have no strong argument in favor of one solution. Both solutions are acceptable. Actually, Jaws behaves like this PR; on the opposite, Narrator reports the dead key immediately.

[amirmahdifard]

@Emil-18 actually, i was having an annoying problem with the input help that i was planning to report it but sadly i didn't have enough time to do it. when input help is onn (personally i use it for teaching to my students and any key presses shouldn't have any effect but only speech about what is that key), wich is ok in all other cases and i don't have any problem, for example, when input help is turned on, any toggle keys i press such as capslock, scrole lock, nvda reports them and they don't have any effect, but i have problem with only the numlock key. when input help is on, pressing the numlock button, toggles it between numlock on and numlock off as normal, which shouldn't be the case. can you possibly resolve it? thanks.

[LeonarddeR]

@CyrilleB79 wrote:

Personally, I was not a big fan of this change

I tend to agree.

You have done the choice not to report dead keys when pressed in input help mode, and to report the modified letter on next press instead. Personally, I would have preferred the other solution, i.e. report dead key immediately and do not modify the next character.

That would also be my preference.

I think I can live with this pr when it would report both the key sequence and the result. For example, report shift+1, bang.

I'd also rather see the magic happen in a helper function, rather than in _handleInputHelp itself. May be it even belongs in keyboardHandler. That already has logic to detect typed cahracters, may be that can be reused (not duplicated)?

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests (tags: installer NVDA).
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/3fi62mm27ylthy06/artifacts/output/nvda_snapshot_pr15907-30301,e5f28533.exe
• CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 28.5, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 16.8, FINISH_END 0.1

See test results for failed build of commit e5f2853305

[Emil-18]

If I don't allow dead keys through like I do now, and the user presses a dead key character, and then turns on input help, it will continue to report characters with the dead key until input help is turned off.

[Emil-18]

@amirmahdifard It is done on purpose. See #4226. I don't think it is possible to fix it

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests (tags: installer NVDA).
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/2whpjjm0mmb6gr3d/artifacts/output/nvda_snapshot_pr15907-30303,4e6b95b8.exe
• CI timing (mins): INIT 0.0, INSTALL_START 0.9, INSTALL_END 1.2, BUILD_START 0.0, BUILD_END 26.8, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 16.7, FINISH_END 0.1

See test results for failed build of commit 4e6b95b85c

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests (tags: installer NVDA).
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/5s5i1imk2efg8n8k/artifacts/output/nvda_snapshot_pr15907-30309,ce5850eb.exe
• CI timing (mins): INIT 0.0, INSTALL_START 0.8, INSTALL_END 1.2, BUILD_START 0.0, BUILD_END 27.0, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 16.5, FINISH_END 0.1

See test results for failed build of commit ce5850eb18

[Emil-18]

@CyrilleB79 If I use 0x4, to my knolage, it is impossible to get information about characters that you have to type a dead key character first, and then the second character to produce. For example, if I type a dead key character, and then e, it would always have reported e regardless if e was another character when the dead key character was typed first. The opisit is also true if I type a dead key character and then turn on input help. The reason why I used 5 is because it is just used in the internalKeyDownEvent function in keyboardHandler in the same way. And should I continue to work on this as #15891 is blocked?

[codeofdusk]

Personally, I was not a big fan of this change

Agree!

[lukaszgo1]

Personally, I was not a big fan of this change

I tend to agree.

Could people who are against this provide some reasoning? For me this makes input help mode closest to the sighted person experience, where they can just glance at a given key to know what additional character can be typed with it.

I think I can live with this pr when it would report both the key sequence and the result. For example, report shift+1, bang.

If this is implemented I'd suggest to revert the order - user knows what the pressed keys do separately, so the new info i.e. the actual typed character should be presented first.

[cary-rowen]

@Emil-18 could you resolve the conflict?

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests (tags: installer NVDA).
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/6pftit7xqmuj3y93/artifacts/output/nvda_snapshot_pr15907-31395,9195eb9c.exe
• CI timing (mins): INIT 0.0, INSTALL_START 1.1, INSTALL_END 1.0, BUILD_START 0.0, BUILD_END 29.0, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.4, FINISH_END 0.2

See test results for failed build of commit 9195eb9c10

[Adriani90]

@Emil-18 are you still working on this? Or should we mark it as abandoned?

[seanbudd]

Closing as abandoned