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

Introduce IUIAutomation5 interface and ability to announce notifications in Windows 10 Fall Creators Update and later #8045

Merged
merged 13 commits into from Mar 28, 2018

Conversation

Projects
None yet
5 participants
@josephsl
Copy link
Collaborator

josephsl commented Feb 28, 2018

Link to issue number:

#7984, #8009

Summary of the issue:

Introduce IUIAutomation5 interface and ability to announce notifications in Windows 10 Fall Creators Update an later

Description of how this pull request fixes the issue:

In Windows 10 Fall Creators Update and later, apps can inform screen readers to announce specific notification text. This is done via IUIAutomationNotificationEventHandler::HandleNotificationEvent, which is part of IUIAutomation5 interface. Since NVDA only uses IUIAutomation3, update UIA subsystem to use IUIAutomation5. Also, introduce ability to use more recent IUIAutomation interfaces on various Windows 10 releases (such as IUIAutomation4 in Anniversary Update/build 14393).

Testing performed:

Testing done via Windows 10 App Essentials add-on: using recent versions of various app (such as Calculator, Microsoft Store and others) that uses this new event to announce texts.

Known issues with pull request:

None so far.

Change log entry:

New feature: In Windows 10 Fall Creators Update and later, NVDA can announce notifications from apps such as Calculator and Windows Store.

josephsl added some commits Feb 23, 2018

UIA: support IUIAutomation5 so notification event tracking can be sup…
…ported. Re #8009.

Also includes #7984: in Windows 10 Version 1709 (Fall Creators Update), UIA notification event is introduced for the benefit of screen readers so they can announct needed text. This is part of UIA 5 interface, which NVDA does not support yet.
The UIA notification event accepts additional parameters such as notification kind, notification processing constant and activit ID. This is useful for controlling how things should be announced and when. For now, the handler will do nothing.
Also, when initializing UIA handler, use UIA 5, and then fall back to UIA 4.
UIA notification event: treat it the same as any other event except f…
…or additional kwargs for event execution. Re #8009.

Treat UIA notification event just like others except for two things:
* A new UIA_notification event will be fired.
* The new event will populate kwargs based on arguments for the actual event handling function (sender, displayString, etc.). This means app modules, global plugins, objects and others must include these kwargs when defining this event.
UIA interface: obtain appropriate IUIAutomation interface based on Wi…
…ndows release. Re #8009.

A new function named getIUIAInterface (along with a private companion for Windows 10) is introduced to obtain highest supported IUIAutomation interface. Depending on Windows release, it'll return various interfaces. In case of Windows 10, a companion function will return appropriate interface based on build ranges.
UIA notification event: add the handler if IUIAutomation5 is in use. Re
#7984.

If IUIAutomation5 is in use, add notificaiton event handler, otherwise COM error is thrown.
Also, instead of relying solely on Python MRO to retrieve UIA interface, save the string somewhere for reference purposes and for future use.
NVDAObjects/UIA: handle UIA notification event from object level. Re #…
…8009.

The base implementation for UIA notification handler will just speak whatever notification it receives. Subclasses are more than welcome to add further processing routines.

@josephsl josephsl requested a review from michaelDCurran Feb 28, 2018

@leonardder

This comment has been minimized.

Copy link
Collaborator

leonardder commented Feb 28, 2018

Hey Joseph,

  1. It seems you defined the getIUIAInterface function, but you never use it in your code below.
  2. As noted on Gitter, I'm honestly not a fan of the build version checking, as it does not work when running from source. Furthermore, I'm not convinced of whether it is significantly faster than just querying interfaces from 5 up to 2. Some timing on my end revealed that querying interfaces is actually very fast.
@michaelDCurran

This comment has been minimized.

Copy link
Contributor

michaelDCurran commented Feb 28, 2018

@michaelDCurran

This comment has been minimized.

Copy link
Contributor

michaelDCurran commented Feb 28, 2018

Do you know if this causes any double speaking where Microsoft may have fired both liveRegionChange and notification for the same thing?

@josephsl

This comment has been minimized.

Copy link
Collaborator Author

josephsl commented Feb 28, 2018

@josephsl

This comment has been minimized.

Copy link
Collaborator Author

josephsl commented Feb 28, 2018

Hi,

@leonardder: the Win10 version is the one to be used from binary builds and eventually from source once we move to Python 3. The generic version is meant for add-ons and to show that NVDA still supports Windows 7 SP1. The reason for this design is to keep it compatible with what we have at the moment: falling back to IUIAutomation2 if newer versions are not available. As @michaelDCurran pointed out, this is meant to be used at startup (both for Core and add-ons).

Thanks.

@michaelDCurran

This comment has been minimized.

Copy link
Contributor

michaelDCurran commented Feb 28, 2018

@josephsl

This comment has been minimized.

Copy link
Collaborator Author

josephsl commented Feb 28, 2018

Hi,

In the end, the getIUIAInterface function won't be added, as a for loop will be used to try various versions (modifying specs accordingly).

Thanks.

UIA interface: query interface from version 5 downwards. Re #8009.
Comments from @leonardder and @michaelDCurran: build ranges aren't useful, subsequently proven to be true. As an alternative, query interfaces from newest to oldest, which does allow NVDA to use IUIAutomation5 interface from source.
Also, this means getIUIAInterface function is no longer necessary.
@leonardder
Copy link
Collaborator

leonardder left a comment

I know you didn't ask for my review, but I'm trying to update my UIA knowledge, so that's why I looked into this.

@@ -1368,6 +1369,16 @@ def event_UIA_systemAlert(self):
# Ideally, we wouldn't use getBrailleTextForProperties directly.
braille.handler.message(braille.getBrailleTextForProperties(name=self.name, role=self.role))

def event_UIA_notification(self, sender=None, notificationKind=None, notificationProcessing=None, displayString=None, activityId=None):

This comment has been minimized.

@leonardder

leonardder Feb 28, 2018

Collaborator

I think you could leave out the sender kwarg. See my comment below.

return
import NVDAObjects.UIA
obj=NVDAObjects.UIA.UIA(UIAElement=sender)
eventHandler.queueEvent("UIA_notification",obj, sender=sender, notificationKind=NotificationKind, notificationProcessing=NotificationProcessing, displayString=displayString, activityId=activityId)

This comment has been minimized.

@leonardder

leonardder Feb 28, 2018

Collaborator

As the sender is equal to obj.UIAElement, I think it is a redundant kwarg. For consistency reasons, if one whould like to access the UIAElement for this event within a subclass, people can use the UIAElement property on the object.

This comment has been minimized.

@michaelDCurran

michaelDCurran Mar 1, 2018

Contributor

I agree with @leonardder here. Sender should not be explicitly provided to the notification event as it is redundant.

self.clientObject=self.clientObject.QueryInterface(IUIAutomation2)
log.info("UIAutomation: %s"%self.clientObject.__class__.__mro__[1].__name__)
# #8009: use appropriate interface based on highest supported interface.
for interface in (IUIAutomation5, IUIAutomation4, IUIAutomation3, IUIAutomation2):

This comment has been minimized.

@leonardder

leonardder Feb 28, 2018

Collaborator

May be put these interfaces in a constant?

except COMError:
pass
# Have this handy because we want to add things corresponding to the interface version.
IUIAVersion = self.clientObject.__class__.__mro__[1].__name__

This comment has been minimized.

@leonardder

leonardder Feb 28, 2018

Collaborator

Now you loop through these interfaces (which I think is a good thing to do), you might as well do something like this, avoiding digging into the mro:

for interface in interfaces:
    QueryInterface
except:
    pass # Or continue? I guess they are identical in result here
else:
    IUIAVersion = interface.__name__
    ...

Since you just left the loop, you can even leave out the else statement, but using an else for the try statement looks somewhat cleaner to me. It's a matter of preference, though.

This comment has been minimized.

@michaelDCurran

michaelDCurran Feb 28, 2018

Contributor

UiaVersion would also have to be set to IUIAutomation when isUIA8 is false. if you do it the way @leonardder suggests

@@ -206,6 +211,9 @@ def MTAThreadFunc(self):
self.clientObject.AddPropertyChangedEventHandler(self.rootElement,TreeScope_Subtree,self.baseCacheRequest,self,UIAPropertyIdsToNVDAEventNames.keys())
for x in UIAEventIdsToNVDAEventNames.iterkeys():
self.clientObject.addAutomationEventHandler(x,self.rootElement,TreeScope_Subtree,self.baseCacheRequest,self)
# #7984: add support for notification event (IUIAutomation5, part of Windows 10 build 16299 and later).
if IUIAVersion >= "IUIAutomation5":

This comment has been minimized.

@leonardder

leonardder Feb 28, 2018

Collaborator

Hmm, even though Python uses lexicographical comparison for strings and therefore this is code that does the write thing, I'd feel safer if you'd avoid this. As @michaelDCurran pointed out in #8045 (comment), all UIA interfaces inherit from the one before, so you could do an instance check on the client object for IUIAutomation5.

self.clientObject=self.clientObject.QueryInterface(interface)
break
except COMError:
pass

This comment has been minimized.

@michaelDCurran

michaelDCurran Feb 28, 2018

Contributor

Perhaps an else on the for loop that raises RuntimeError("No UIAutomation client interface supported")

except COMError:
pass
# Have this handy because we want to add things corresponding to the interface version.
IUIAVersion = self.clientObject.__class__.__mro__[1].__name__

This comment has been minimized.

@michaelDCurran

michaelDCurran Feb 28, 2018

Contributor

UiaVersion would also have to be set to IUIAutomation when isUIA8 is false. if you do it the way @leonardder suggests

josephsl added some commits Mar 1, 2018

@josephsl

This comment has been minimized.

Copy link
Collaborator Author

josephsl commented Mar 1, 2018

Hi,

In the end, isinstance checking will be done. I don't think an "else" is necessary except if we need to break out of the loop with that method.

Additional testing done by running the launcher (based on source code commits) on a Windows 8.1 system, and things work as expected. Thanks.

@Brian1Gaff

This comment has been minimized.

Copy link

Brian1Gaff commented Mar 1, 2018

@josephsl

This comment has been minimized.

Copy link
Collaborator Author

josephsl commented Mar 1, 2018

Hi,

If the new interface includes useful features that screen reader users need to see/hear, then yes, we need to add support for it.

Thanks.

return
import NVDAObjects.UIA
obj=NVDAObjects.UIA.UIA(UIAElement=sender)
eventHandler.queueEvent("UIA_notification",obj, sender=sender, notificationKind=NotificationKind, notificationProcessing=NotificationProcessing, displayString=displayString, activityId=activityId)

This comment has been minimized.

@michaelDCurran

michaelDCurran Mar 1, 2018

Contributor

I agree with @leonardder here. Sender should not be explicitly provided to the notification event as it is redundant.

UIA notification: do not include sender argument. Re #8009.
Reviewed by @michaelDCurran (NV Access): sender argument is redundant.

michaelDCurran added a commit that referenced this pull request Mar 5, 2018

Merge branch 'master' into iuiautomation5
# Conflicts:
#	source/NVDAObjects/UIA/__init__.py
#	source/_UIAHandler.py

michaelDCurran added a commit that referenced this pull request Mar 11, 2018

@michaelDCurran michaelDCurran merged commit 35c0840 into nvaccess:master Mar 28, 2018

@nvaccessAuto nvaccessAuto added this to the 2018.2 milestone Mar 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.