Repository navigation
Speak typed characters in windows consoles in the correct language - #10551
Closed
michaelDCurran wants to merge 2 commits into
Closed
michaelDCurran wants to merge 2 commits into
michaelDCurran wants to merge 2 commits into
Conversation
…characters in Windows consoles. As before, first try getting the keyboard layout of the focus (input) thread. If that fails, get the last keyboard layout change detected by NVDA on the foreground object, Finally, fallback to the keyboard layout of NVDA's main thread.
LeonarddeR
reviewed
Nov 27, 2019
| # Cache the new keyboard layout on the foreground object, | ||
| # Which may be used by keyboardHandler | ||
| # when speaking typed characters in situations where it cannot be fetched directly from the focus. | ||
| fg._lastDetectedKeyboardLayoutChange = hkl |
Collaborator
There was a problem hiding this comment.
I'd suggest storing this in globalVars instead.
| hkl = getattr(api.getForegroundObject(), '_lastDetectedKeyboardLayoutChange', 0) | ||
| if not hkl: | ||
| log.debug("No layout cached, falling back to layout of NVDA main thread") | ||
| # As a last resort, use the keyboard layout of NVDA's main thread. |
Collaborator
There was a problem hiding this comment.
How likely would it be that the keyboard layout for the NVDA main thread differs from the keyboard layout of the currently focused app?
Member
Author
|
In Windows 7, input language changes were per GUI thread.
In Windows 10, the default is now that input language changes apply
across the entire desktop. However, this can be configured to apply
per-app again, in Settings -> System -> time and language -> keyboard
language settings.
If that option didn't exist, we could just use the thread of the shell
window, or perhaps NVDA's main thread -- though it does seem that NvDA's
UI needs to get focus before it picks up a desktop-wide input language
change.
Since it can be per-app, we need to track keyboard layouts per thread,
or at very least, per app (process ID).
Now thinking about it, I think it will be safe enough to cache it on the
appModule.
However, the real problem here is that the inputLangChange notification
for console windows comes from a conhost thread, where as Windows has
hacked ConsoleWindowClass windows to reflect the thread and processID of
the attached process requesting input (I.e. Ubuntu bash). We could work
out the conhost processID from the thread ID and then get the appModule
for that and cache the layout there. But, the other day I created /
merged a pr that deliberately forced the processID property for
winConsole UIA NVDAObject's to again reflect the process ID of the
hosted app, not conhost. These two ideas are in conflict with each other.
I'll have to think about this further...
|
Collaborator
|
Thanks for the explanation. Because it's problematic, that's why I suggested globalVars. We only have to cache just one layout anyway. My personal belief is that caching things on objects should be avoided. |
Member
Author
|
But if we cache on globalVars, then we can only track one layout. And if the user has configured for per-app input language changes, this will cause us to sometimes use the wrong layout again. I think the better approach here is going to be finding a way to get the real input thread of conhost and passing that to GetKeyboardLayout like we do for Edge and such. |
Contributor
|
This may also fix the dead key bug mentioned in #9915. |
Member
Author
|
Replaced by pr #10554 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Fixes #10113
Summary of the issue:
With the work to support Windows consoles via UIA, NVDA moved to speaking typed characters in consoles by detecting keyboard input and then using ToUnicodeEx to convert the keyboard input into a character for speaking. We had already been taking this approach for Microsoft Edge and other places in Windows 10 where we could not inject in-process.
ToUnicodeEx requires a keyboard layout to use the correct language. Therefore, we would call GetKeyboardLayout, giving it the thread ID of the focused window.
However, it seems that Windows console windows report the wrong thread ID, and therefore, GetKeyboardLayout fails, returning 0 instead of a valid keyboard layout.
Passing this 0 to ToUnicodeEx, it would generate characters in the user's system language (I think). At least, on my system it was always english US no matter what input language I was currently using.
Description of how this pull request fixes the issue:
We now try multiple things to get a valid keyboard layout:
Testing performed:
Known issues with pull request:
None.
Change log entry:
None