Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speak typed characters in UWP apps on Windows 10 RS2 and above #6631

Merged
merged 6 commits into from Jan 23, 2017

Conversation

michaelDCurran
Copy link
Member

Fixes #6017

This code will only work on Windows build 14986 and above.

@@ -162,6 +167,21 @@ def internal_keyDownEvent(vkCode,scanCode,extended,injected):
return False
except:
log.error("internal_keyDownEvent", exc_info=True)
finally:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth commenting why we do this in the finally block; i.e. because we return early in several places but we still want this to be executed.

finally:
# #6017: handle typed characters for UWP apps in Win10 RS2 and above where we can't detect typed characters in-process
focus=api.getFocusObject()
if sys.getwindowsversion().build>=14986 and not gestureExecuted and not isNVDAModifierKey(vkCode,extended) and not vkCode in KeyboardInputGesture.NORMAL_MODIFIER_KEYS and focus.windowClassName.startswith('Windows.UI.Core'):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling sys.getwindowsversion probably isn't expensive, but we already have it cached in winVersion.winVersion anyway, so I'd use that instead.

keyStates[k]=ctypes.windll.user32.GetKeyState(k)
charBuf=ctypes.create_unicode_buffer(5)
# Normally calling ToUnicodeEx would destroy keyboard buffer state and therefore cause the app to not produce the right WM_CHAR message.
# However in UWP apps in Windows 10 RS2 and above, wm_keydown / wm_char is never processed, thus NVDA will be the only one calling it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment still true? I thought the point of the flag was to stop this state destroying behaviour, rather than it being something specific to UWP.

# Normally calling ToUnicodeEx would destroy keyboard buffer state and therefore cause the app to not produce the right WM_CHAR message.
# However in UWP apps in Windows 10 RS2 and above, wm_keydown / wm_char is never processed, thus NVDA will be the only one calling it.
hkl=ctypes.windll.user32.GetKeyboardLayout(focus.windowThreadID)
res=ctypes.windll.user32.ToUnicodeEx(vkCode,scanCode,keyStates,charBuf,5,4,hkl)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I assume 4 is the flag? Please comment as to what the 4 does.
  2. I assume 5 is the size? Perhaps use len(charBuf) to be clearer and so we don't break things if we change the size in future.

res=ctypes.windll.user32.ToUnicodeEx(vkCode,scanCode,keyStates,charBuf,5,4,hkl)
if res>0:
for ch in charBuf[:res]:
eventHandler.queueEvent("typedCharacter",focus,ch=ch)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this saying there can be multiple separate characters? I know there are languages where a single key press produces a conjunct Unicode character sequence (e.g. Tamil), but we probably actually want that to be handled as one character. We can't do that with WM_CHAR right now, but is there a reason we shouldn't do this here since we can?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have certainly seen it produce at least 2 characters yes. For instance, if a dead key is hit, followed by a character that is not compatible with a dead key, then you get a char for the dead key (say ^) and also the actual character.

@michaelDCurran
Copy link
Member Author

Actually, technically the flag is only needed for non-uwp apps as TranslateMessage is never called. So technically, we could just call it with a flag of 0 and that comment would be correct. However, if we were to ever extend the use of this code to outside of UWP apps, then the flag is most certainly needed. The actual mS bug I filed was to ensure that the flag produced the same results whether it was UWP or not. As in older Windows builds, the flag in UWP apps breaks things just as badly as if ToUnicodeEx were used in conjunction with TranslateMessage, and no flag given.

@Nikita34196
Copy link

When will this fix in nvda next? and will be in the release 2017.1

@derekriemer
Copy link
Collaborator

derekriemer commented Dec 25, 2016 via email

@josephsl
Copy link
Collaborator

josephsl commented Dec 25, 2016 via email

@michaelDCurran
Copy link
Member Author

@jcsteh: I have made all requested changes.

michaelDCurran added a commit that referenced this pull request Jan 9, 2017
@josephsl
Copy link
Collaborator

josephsl commented Jan 9, 2017

Hi,

Critical bug found: After installing next.13789, try typing into Start menu or UWP apps. At least in build 14986 (my laptop), I get a traceback that says:

ERROR - unhandled exception (19:55:28):
Traceback (most recent call last):
File "_ctypes/callbacks.c", line 315, in 'calling callback function'
File "winInputHook.pyc", line 45, in keyboardHook
File "keyboardHandler.pyc", line 183, in internal_keyDownEvent
WindowsError: exception: access violation writing 0x00000005

Same result when I try to type an address into Edge. This was with speak typed chars set to off. Thanks.

@michaelDCurran
Copy link
Member Author

michaelDCurran commented Jan 9, 2017 via email

michaelDCurran added a commit that referenced this pull request Jan 9, 2017
@josephsl
Copy link
Collaborator

Hi,

Try the following:

  1. From build 15014 running on a laptop, go to Settings/Network/Status.
  2. Open Show Available networks.
  3. Press TAB to go through the network flyout until you arrive at the list of networks.
  4. Select one, then press TAB to go to either Properties or Connect/Disconnect.
  5. Press Escape.

Expected: no errors.
Actual:

@josephsl
Copy link
Collaborator

Hi,

Try the following:

  1. On a laptop running build 15014, go to Settings/Network/Status.
  2. Go to Show Available networks.
  3. Select a wireless network and tab to either Properties or connect/disconnect.
  4. Press Escape.

Expected: no errors.
Actual:

ERROR - eventHandler.executeEvent (15:47:46):
error executing event: typedCharacter on <NVDAObjects.UIA.UIA object at 0x050A8ED0> with extra args of {'ch': u'\x1b'}
Traceback (most recent call last):
File "eventHandler.pyc", line 143, in executeEvent
File "eventHandler.pyc", line 91, in init
File "eventHandler.pyc", line 98, in next
File "NVDAObjects_init_.pyc", line 840, in event_typedCharacter
File "speech.pyc", line 627, in speakTypedCharacters
File "api.pyc", line 235, in isTypingProtected
File "baseObject.pyc", line 34, in get
File "baseObject.pyc", line 110, in getPropertyViaCache
File "NVDAObjects\UIA_init
.pyc", line 801, in _get_states
File "baseObject.pyc", line 34, in get
File "baseObject.pyc", line 110, in getPropertyViaCache
File "NVDAObjects\UIA_init
.pyc", line 797, in _get_UIACachedStatesElement
COMError: (-2147467259, 'Unspecified error', (None, None, None, 0, None))

This happens on next.13789 and later, thus I suspect this has to do with this pull request.

Thanks.

@michaelDCurran
Copy link
Member Author

@josephsl I'd say that if speak typed characters had originally worked in UWP apps without this code, we'd see the same errors here. As they don't impact on usage, I won't hold back this PR. However we may consider cleaning up the way errors in api.isprotected are handled, as this does seem to happen quite a bit.

@michaelDCurran michaelDCurran merged commit 6406413 into master Jan 23, 2017
@nvaccessAuto nvaccessAuto added this to the 2017.1 milestone Jan 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants