Report live region changes outside of web content (E.g. settings search messages, Windows update progress on logon, and Skype for business chat messages etc)#7287
Merged
Conversation
…tent. UIA's liveRegionChanged event, and MSAA's object_liveRegionChanged event are mapped to a new liveRegionChange event in NVDA. the base NVDAObject handles this event by simply reporting the name of the object in speech and braille.
…has custom live region support via virtualBuffers that is much more accurate.
feerrenrut
suggested changes
Jun 19, 2017
| content=self.value.replace('\r','\n').strip() | ||
| fullText=self.name.replace('\r\n\r\n','\n') | ||
| contentLines=content.split('\n') | ||
| contentStartIndex=fullText.find(content) |
Contributor
There was a problem hiding this comment.
So if I'm understanding this right, self.name contains self.value ?
Contributor
There was a problem hiding this comment.
I think it might be handy to have some examples of what these fields might look like before the processing is done. Or perhaps unit tests?
| pretext=fullText[:contentStartIndex] | ||
| # There are some annoying comma characters after the person's name | ||
| pretext=pretext.replace(' ,','') | ||
| # If the object's are the same, the person is the asame, and the new content is the old content but with more appended, report the appended content |
Contributor
There was a problem hiding this comment.
typo: asame should be same
|
|
||
| class AppModule(appModuleHandler.AppModule): | ||
|
|
||
| _lastLiveChatMessageData=[],"",[] |
Contributor
There was a problem hiding this comment.
Probably worth having a docstring to describe what each of these entries are.
feerrenrut
suggested changes
Jun 20, 2017
| # Example string: "Michael Curran : , , Hello\nThis is a test , 10:45 am." | ||
| # Where person is "Michael Curran", content is "Hello\nThis is a test" and timestamp is "10:45 am" | ||
| # The object's value just contains the content. | ||
| # Example: "Hello\nThis is a test" |
Contributor
There was a problem hiding this comment.
Maybe include in these examples the \r\n details. To show what the raw strings we get are like.
feerrenrut
approved these changes
Jun 20, 2017
Contributor
|
Hi, An important issue found: in some apps such as Maps app in Windows 10, the app keeps firing live region change event even when the content hasn't changed. At the moment my workaround is to store the last spoken message as part of the app module and announce new content if the conditions are right (current message isn't the same as the last one known). Thanks. |
Member
Author
|
If you could submit a pr specifically for maps that would be great :)
Really repeating or inappropriate live regions are bugs and should be
reported to Microsoft. I would not want to add general code to NVDA to
try and protect against this, as the majority of live regions should
behave okay. However, am happy to see specific fixes for Maps.
|
Contributor
|
Hi, actually, Maps is just one of the examples. The other prominent examples are Windows Store and Settings. In case of Settings, it requires more work, as the way certain things – especially Windows Update download and installation status – are different between stable and Insider Preview builds. In case of stable builds, another problem is that the download and installation progress is announced despite the fact that the progress bar for it is next door and fires value change events. Thanks.
From: Michael Curran [mailto:notifications@github.com]
Sent: Saturday, June 24, 2017 6:26 PM
To: nvaccess/nvda <nvda@noreply.github.com>
Cc: Joseph Lee <joseph.lee22590@gmail.com>; Comment <comment@noreply.github.com>
Subject: Re: [nvaccess/nvda] Report live region changes outside of web content (E.g. settings search messages, Windows update progress on logon, and Skype for business chat messages etc) (#7287)
If you could submit a pr specifically for maps that would be great :)
Really repeating or inappropriate live regions are bugs and should be
reported to Microsoft. I would not want to add general code to NVDA to
try and protect against this, as the majority of live regions should
behave okay. However, am happy to see specific fixes for Maps.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#7287 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkEDWsH0RbR-Wjsxv1cOm9wlmL5AXks5sHbc9gaJpZM4N6w6s> .
|
Member
Author
|
That is fair enough.
If you're willing to look at fixes for these, I'm happy to review PRs
for however you think these things need to be fixed. At this stage going
straight into core would be preferable.
|
Contributor
|
Hi, I think a simpler solution for now is introducing an attribute named “lastLiveText” into NVDAObjects that live region change can look up when announcing things. If you’d like me to add this, please let me know so I can submit a PR for this PR. Thanks.
From: Michael Curran [mailto:notifications@github.com]
Sent: Saturday, June 24, 2017 7:18 PM
To: nvaccess/nvda <nvda@noreply.github.com>
Cc: Joseph Lee <joseph.lee22590@gmail.com>; Comment <comment@noreply.github.com>
Subject: Re: [nvaccess/nvda] Report live region changes outside of web content (E.g. settings search messages, Windows update progress on logon, and Skype for business chat messages etc) (#7287)
That is fair enough.
If you're willing to look at fixes for these, I'm happy to review PRs
for however you think these things need to be fixed. At this stage going
straight into core would be preferable.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#7287 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkLWLJAtLlMhM5f1XsVBtzcuXxGpcks5sHcNTgaJpZM4N6w6s> .
|
Member
Author
|
It would be also good to track the UIA runtime ID and only speak if both
the runtime ID and text are different. Then for truly different
elements, the live region will not be suppressed just because they
happened to have the same text.
Perhaps a dictionary of runtimeIDs to last live text?
Hold off on this for now. I'll have a good think when I'm back on Monday
about how we should best handle this.
|
Member
Author
|
Could you please provide some exact steps to reproduce?
At least one in maps, and one somewhere else.
Thanks
|
Contributor
|
Hi, Try it with Windows 10 App Essentials add-on disabled: Maps:
Expected: no repeats as to the current map view. Windows Store:
Expected: no repeated announcements. Thanks. |
Member
Author
|
@josephsl: Do you know if the duplicate live region change events happen in Creaters update 15063? I am now finally seeing them in 16232, but was not seeing them in 16215, so am wondering if they were a recent bug. If so, then we have more time to report it to Microsoft. Mind you, I'm also seeing crashes in maps now which is making this hard to debug. |
Contributor
|
Hi, this is build agnostic – it’ll happen on 15063 as well as on newer builds, as it is mostly an app thing. Thanks.
From: Michael Curran [mailto:notifications@github.com]
Sent: Tuesday, July 11, 2017 12:00 AM
To: nvaccess/nvda <nvda@noreply.github.com>
Cc: Joseph Lee <joseph.lee22590@gmail.com>; Mention <mention@noreply.github.com>
Subject: Re: [nvaccess/nvda] Report live region changes outside of web content (E.g. settings search messages, Windows update progress on logon, and Skype for business chat messages etc) (#7287)
@josephsl <https://github.com/josephsl> : Do you know if the duplicate live region change events happen in Creaters update 15063? I am now finally seeing them in 16232, but was not seeing them in 16215, so am wondering if they were a recent bug. If so, then we have more time to report it to Microsoft. Mind you, I'm also seeing crashes in maps now which is making this hard to debug.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#7287 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkHYbQKv1lYpfrU-G7ya7Cz04ctLgks5sMx2EgaJpZM4N6w6s> .
|
…text within a short time of each other. Multiple apps in windows 10 (Maps, Store etc) have buggy events. No runtimeID check can be made here as in many cases multiple elements in a list (E.g. store updates for each app) are firing all at the same time. * Added a _shouldAllowUIALiveRegionChangeEvent property to UIA NVDAObject. This is private as it may be changed or removed in the future when live region events are rethought in Windows 10. this property compairs the text and time against the last liveRegion change.
Member
Author
jcsteh
approved these changes
Jul 19, 2017
jcsteh
left a comment
Contributor
There was a problem hiding this comment.
No runtimeID check can be made here as in many cases multiple elements in a list (E.g. store updates for each app) are firing all at the same time.
Do you mean multiple list items all have a live region containing the same text?
| newText=self.name | ||
| newTime=time.time() | ||
| if newText==oldText and oldTime is not None and (newTime-oldTime)<0.5: | ||
| return False |
Contributor
There was a problem hiding this comment.
Do we still want to update the cached time if the text is equal? Unless I'm missing something, this will still spam every half second if the same text keeps firing.
| return True | ||
|
|
||
| _lastLiveRegionChangeInfo=(None,None) #: Keeps track of the last live region change (text, time) | ||
| def _get__shouldAllowUIALiveRegionChangeEvent(self): |
Contributor
There was a problem hiding this comment.
It'd be good to have a brief comment here explaining what this does, similar to your commit summary. Not that the code is complex, but super clarity is nice. :)
Contributor
|
Hi, for Store, yes, each list item raises live region change event, most evident when installing multiple app updates at once (at least early and late phases). Thanks.
From: James Teh [mailto:notifications@github.com]
Sent: Tuesday, July 18, 2017 9:24 PM
To: nvaccess/nvda <nvda@noreply.github.com>
Cc: Joseph Lee <joseph.lee22590@gmail.com>; Mention <mention@noreply.github.com>
Subject: Re: [nvaccess/nvda] Report live region changes outside of web content (E.g. settings search messages, Windows update progress on logon, and Skype for business chat messages etc) (#7287)
@jcsteh approved this pull request.
No runtimeID check can be made here as in many cases multiple elements in a list (E.g. store updates for each app) are firing all at the same time.
Do you mean multiple list items all have a live region containing the same text?
_____
In source/NVDAObjects/UIA/__init__.py <#7287 (comment)> :
@@ -824,6 +825,16 @@ def _get_shouldAllowUIAFocusEvent(self):
except COMError:
return True
+ _lastLiveRegionChangeInfo=(None,None) #: Keeps track of the last live region change (text, time)
+ def _get__shouldAllowUIALiveRegionChangeEvent(self):
+ oldText,oldTime=self._lastLiveRegionChangeInfo
+ newText=self.name
+ newTime=time.time()
+ if newText==oldText and oldTime is not None and (newTime-oldTime)<0.5:
+ return False
Do we still want to update the cached time if the text is equal? Unless I'm missing something, this will still spam every half second if the same text keeps firing.
_____
In source/NVDAObjects/UIA/__init__.py <#7287 (comment)> :
@@ -824,6 +825,16 @@ def _get_shouldAllowUIAFocusEvent(self):
except COMError:
return True
+ _lastLiveRegionChangeInfo=(None,None) #: Keeps track of the last live region change (text, time)
+ def _get__shouldAllowUIALiveRegionChangeEvent(self):
It'd be good to have a brief comment here explaining what this does, similar to your commit summary. Not that the code is complex, but super clarity is nice. :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#7287 (review)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkKLh7XBWviAW69-UBYQdqV_UIl25ks5sPYTqgaJpZM4N6w6s> .
|
michaelDCurran
added a commit
that referenced
this pull request
Jul 19, 2017
jcsteh
approved these changes
Jul 19, 2017
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
5 participants
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 #7286
This PR implements support for live regions outside of web content by listening to both UIA liveRegionChanged and MSAA object_liveRegionChanged events, and mapping them to event_liveRegionChange.
A base implementation has been provided on the base NVDAObject, which just reports the name of the object when the event is fired. This is enough for Skype store app, Settings search messages (I.e. "no search results").
An appModule has been also provided for Skype for Business which has a custom implementation of event_liveRegionChange, which works around Skype for Business's strange way of concatinating new messages from the same person in the same minute.
The PR also stubs out event_liveRegionChange on the base MSHTML NVDAObject, as MSHTML live regions are already handled much better via custom in-process code.