UI Automation in Windows Console: limit blank lines in review and initial word movement support #9647

codeofdusk · 2019-05-31T10:11:43Z

Link to issue number:

Split from #9646 (builds on #9614).

Summary of the issue:

Currently, in consoles with UI Automation enabled:

In object review, commands for moving to and reporting words move to or report lines instead.
A large number (thousands in many cases) of empty lines are appended to the console's object review.

Description of how this pull request fixes the issue:

Word movement and blank line filtering have been implemented in an overridden move method on consoleUIATextInfo. Since the console's UIA implementation does not provide word movement, we must use an offset-based word movement algorithm which uses the Uniscribe API to find word boundaries. The algorithm used for this implementation is based on that in NVDAObjects.offsets._getWordOffsets. Blank lines are filtered by comparing the textInfo's _rangeObj to the last visible text range (returned by winConsoleUIA.UIATextPattern.GetVisibleRanges).

Testing performed:

Tested text review on Windows 10 versions 1803 and 1903.

Known issues with pull request:

While "review previous word" and "review next word" now allow word movement, the cursor is not consistently placed on the first character of the word under review.

See #9646 for all console UIA known issues to date.

Change log entry:

None.

Note: there are issues when reaching the bottom of the review (cursor gets stuck).

This should mostly restore caret movement support.

…xtInfo._getCurrentOffset. The reverse option has been removed.

codeofdusk · 2019-05-31T11:13:10Z

CC @feerrenrut, @LeonarddeR, @michaelDCurran.

michaelDCurran · 2019-06-02T23:40:03Z

source/NVDAObjects/UIA/winConsoleUIA.py

@@ -39,7 +153,10 @@ class winConsoleUIA(Terminal):

 	def _reportNewText(self, line):
 		# Additional typed character filtering beyond that in LiveText
-		if self._isTyping and time.time() - self._lastCharTime <= self._TYPING_TIMEOUT:
+		if (


Even if this is just fixing up code formatting, I don't think this is directly related to this pr, so I'd prefer to see it in one of the other ones that deals directly with speakTypedWords etc.

michaelDCurran · 2019-06-02T23:42:52Z

source/NVDAObjects/UIA/winConsoleUIA.py

+			# Insure we haven't gone beyond the visible text.
+			# UIA adds thousands of blank lines to the end of the console.
+			visiRanges = self.obj.UIATextPattern.GetVisibleRanges()
+			lastVisiRange = visiRanges.GetElement(visiRanges.length - 1)


It may be possible that visiRanges has a length of 0. It would probably be a bug, but best to check anyway. Only do the next lot of code if length>0.

michaelDCurran · 2019-06-02T23:44:52Z

source/NVDAObjects/UIA/winConsoleUIA.py

+			# UIA adds thousands of blank lines to the end of the console.
+			visiRanges = self.obj.UIATextPattern.GetVisibleRanges()
+			lastVisiRange = visiRanges.GetElement(visiRanges.length - 1)
+			if self._rangeObj.CompareEndPoints(


What about also checking that we are not before the first visible range? I.e. we should not allow someone to move off the top of the screen up into previous content.

michaelDCurran · 2019-06-02T23:50:34Z

source/NVDAObjects/UIA/winConsoleUIA.py

+			# UIA adds thousands of blank lines to the end of the console.
+			visiRanges = self.obj.UIATextPattern.GetVisibleRanges()
+			lastVisiRange = visiRanges.GetElement(visiRanges.length - 1)
+			if self._rangeObj.CompareEndPoints(


I think I would prefer to do this check after the call to super, and then restore the textRange if we have moved too far. If my logic is correct, I'm pretty sure that this current code will still allow moving to the first line after the last visible range? Because when on the last line, compareEndPoints textInfo's start with lastVisiRange's end would not yet be >=0.
So before super, I'd save off a copy of _rangeObj with something like:

oldRange=self._rangeObj.clone()

Then after super, do these checks, and if we have moved out of range, then replace self._rangeObj with oldRange, and return 0.

michaelDCurran · 2019-06-02T23:53:49Z

source/NVDAObjects/UIA/winConsoleUIA.py

+				return 0
+		if unit == textInfos.UNIT_WORD and direction != 0:
+			# UIA doesn't implement word movement, so we need to do it manually.
+			offset = self._getCurrentOffset()


I'd like to see this method better named to reflect that this is talking about within the current line. Something like _getCurrentOffsetInCurrentLine.

michaelDCurran · 2019-06-02T23:55:44Z

source/NVDAObjects/UIA/winConsoleUIA.py

+			# UIA doesn't implement word movement, so we need to do it manually.
+			offset = self._getCurrentOffset()
+			index = 1 if direction > 0 else 0
+			start, end = self._getWordOffsets(offset)


Again: perhaps call it _getWordOffsetsInCurrentLine

michaelDCurran · 2019-06-02T23:58:05Z

source/NVDAObjects/UIA/winConsoleUIA.py

+			if res != 0:
+				return direction
+			else:
+				if self.move(textInfos.UNIT_CHARACTER, -1):


Can you clarify what this does exactly and why it is necessary?

michaelDCurran · 2019-06-03T00:03:52Z

source/NVDAObjects/UIA/winConsoleUIA.py

+		return super(consoleUIATextInfo, self).move(unit, direction, endPoint)
+
+	def expand(self, unit):
+		if unit == textInfos.UNIT_WORD:


Expand should use similar code to move. E.g.:
If unit is word, then use _getCurrentOffset and _getWordOffsets and manually move the start and end of the range to have it correctly bound the current word.
Then, there is no need to change what text is exposed as the textRange will be correct.
The advantage of this is there is less to track, it is easier to read, and most importantly, later comparisons or movements of this textInfo won't behave like it is expanded to line even though the text returned is word.
There may be a question around performance: but I'd like to see the code changed to my proposed way before we then consider if optimisations are needed at all.

Note: this new approach includes characters after a line break in the current word.

michaelDCurran · 2019-06-03T23:40:01Z

source/NVDAObjects/UIA/winConsoleUIA.py

@@ -140,8 +151,8 @@ def _getCurrentWord(self):
 		lineInfo = self.copy()


I think _getCurrentWord can be removed now as it is no longer used?

michaelDCurran · 2019-06-03T23:42:34Z

source/NVDAObjects/UIA/winConsoleUIA.py

+			self._rangeObj.MoveEndpointByUnit(
+				UIAHandler.TextPatternRangeEndpoint_Start,
+				UIAHandler.NVDAUnitsToUIAUnits[textInfos.UNIT_CHARACTER],
+				wordEndPoints[0]


What happens if wordEndPoints[0] or wordEndPoints[1] is 0? I.e. you don't ned to move? perhaps these moveByUnit calls should be within an if conndition checking that wordEndPoints[x] is not 0.

michaelDCurran · 2019-06-04T03:03:35Z

when moving by word, the first two characters of the next line are included in the last word on a line.
The reason for this is to do with the fact that an extra two alphanumeric characters are placed at the end of the text with give to calculateWordOffsets to get around a uniscribe bug. However, when returning the word offsets, len(lineText) is used. This should be len(lineText)-2 as we don't want to include the inserted characters.

michaelDCurran · 2019-06-04T03:04:46Z

master will need to be merged into this branch, and any conflicts resolved.

codeofdusk · 2019-06-04T05:24:43Z

@michaelDCurran It's almost perfect now (I think the word movement bugs we were talking about earlier are fixed by the new implementation of expand). Only problem now is when you move past the last word on a line, NVDA repeatedly says "blank" (moving to the previous word puts us on the last line though).

LeonarddeR · 2019-06-04T06:08:44Z

source/NVDAObjects/UIA/winConsoleUIA.py

@@ -17,10 +19,12 @@

 class consoleUIATextInfo(UIATextInfo):
 	_expandCollapseBeforeReview = False
+	_isCaret = False


I don't think this is necessary, see below.

LeonarddeR · 2019-06-04T06:10:43Z

source/NVDAObjects/UIA/winConsoleUIA.py

@@ -31,6 +35,121 @@ def __init__(self, obj, position, _rangeObj=None):
 					1
 				)

+	def move(self, unit, direction, endPoint=None):
+		oldRange=None
+		if not self._isCaret:


Note that every TextInfo instance has an attribute basePosition that should be textInfos.POSITION_CARET. Therefore, I think I'd prefer this:

Suggested change

if not self._isCaret:

if self.basePosition != textInfos.POSITION_CARET:

LeonarddeR · 2019-06-04T06:13:01Z

source/NVDAObjects/UIA/winConsoleUIA.py

+			# Insure we haven't gone beyond the visible text.
+			# UIA adds thousands of blank lines to the end of the console.
+			visiRanges = self.obj.UIATextPattern.GetVisibleRanges()
+			if visiRanges.length > 0:


I don't know how caching works here, but it might make sense to do something like this:

Suggested change

if visiRanges.length > 0:

visibleRangesLength = visiRanges.Length

if visibleRangesLength > 0:

What exactly is the advantage of doing this? We only use the value once per function call.

You're also getting the length for lastVisiRange

source/NVDAObjects/UIA/winConsoleUIA.py

LeonarddeR · 2019-06-04T06:27:49Z

source/NVDAObjects/UIA/winConsoleUIA.py

+		charInfo = self.copy()
+		res = 0
+		chars = None
+		while True:


Recent experiences with Notepad++ made me a bit worried about endless while loops. Could you limit this somehow?

Would something like this work?

Suggested change

while True:

while charInfo.compareEndPoints(

lineInfo,

"startToEnd"

) <= 0:

LeonarddeR · 2019-06-04T06:33:22Z

source/NVDAObjects/UIA/winConsoleUIA.py

+		else:
+			return super(consoleUIATextInfo, self).expand(unit)
+
+	def _getCurrentOffsetInThisLine(self):


In most if not all cases, you're calling _getCurrentOffsetInThisLine and then _getWordOffsetsInThisLine, which both copy self and expand to a line. May be lineInfo could be an argument to both functions to avoid this? Of course, it should still only be used when self is collapsed within that line

michaelDCurran

When moving forward by word and you land on a blank line, moving forward by word again gets stuck on the blank line. The reason for this is that although UIA textRange's treat a blank line as one character unit, the text it gives back for this unit is of 0 length. Therefore getWordOffsetsInThisLine ends up giving back 0,0.
When fetching the line text in getWordOffsetInThisLine, set lineText to u" " if lineText is of 0 length. Easiest way is to do:

lineText=info.text or u" "

codeofdusk · 2019-06-05T07:22:40Z

@michaelDCurran Thanks. Now, when moving forward by word, we can land (and pass over) the blank, but moving back by word skips over it. I think this is pretty minor though and this PR is ready for another review/merge.

…nger jump over the last word on the previous line.

michaelDCurran · 2019-06-05T12:27:27Z

I think I have come up with a fix for moving previous word that does not skip the last word of the previous line.
You can find the code in the pr9647_fixMovePreviousWord branch on nvaccess/nvda repository.
I have provided comments through out the code so you can hopefully understand the logic.
It was rather tricky to get right. Essentially it no lonver recursively calls move within the algorithm as in some cases that was happening too many times, plus the steps are now broken down into very small / easy to understand parts.
In short the steps are now:

Relative to the current line, calculate our offset and the current word's offsets.
If moving in a forward direction, we can just jump to the end offset of the current word and we're done.
However, if moving backwards:
- If we are after the beginning of a word, first move back to the start of the word.
- Try to move one character back before the start of the word.
- If we have moved on to a previous line, recalculate our new offset for the new line.
- Finally using the offset, Calculate the current word offsets and move to the start of this word if we are not already there.

michaelDCurran · 2019-06-05T12:30:53Z

If you are happy with the code, and can follow the logic okay, please merge it into this pr and I will then approve and merge this pr to master.

michaelDCurran · 2019-06-05T12:33:19Z

This would have been a lot easier to write just using expand and collapse calls within move, but Microsoft's UIA implementation for consoles seems to not handle collapsing properly, as you found with the review cursor scripts previously.

… cmduia2-textinfos

codeofdusk · 2019-06-05T16:38:53Z

@michaelDCurran Looks good to me!

codeofdusk changed the title ~~UI Automation in Windows Console: only enable UIA when available~~ UI Automation in Windows Console: limit blank lines in review and initial word movement support May 31, 2019

codeofdusk added 6 commits May 31, 2019 06:15

Restrict the number of blank lines shown in console UIA's object review.

41d4816

Note: there are issues when reaching the bottom of the review (cursor gets stuck).

UIA console: only apply our custom move logic when not moving the caret.

53ecca9

This should mostly restore caret movement support.

Add basic word movement support.

a9575ba

ConsoleUIATextInfo: support expantion by word.

1cf0563

consoleUIATextInfo._countCharsToEnd(reverse=True) is now consoleUIATe…

6083ded

…xtInfo._getCurrentOffset. The reverse option has been removed.

Cleanup.

768fe0f

codeofdusk force-pushed the cmduia2-textinfos branch from 6a3f7e3 to 768fe0f Compare May 31, 2019 10:30

Improve cursor placement after word movement.

9a8835c

michaelDCurran requested changes Jun 3, 2019

View reviewed changes

codeofdusk added 2 commits June 3, 2019 19:02

Review actions (without changes to expand)

bc55777

Re-implement expand.

6f52524

Note: this new approach includes characters after a line break in the current word.

michaelDCurran requested changes Jun 3, 2019

View reviewed changes

codeofdusk added 2 commits June 4, 2019 01:19

Review actions.

c2386b9

Merge branch 'master' into cmduia2-textinfos

c016ab1

LeonarddeR reviewed Jun 4, 2019

View reviewed changes

codeofdusk added 3 commits June 4, 2019 03:38

Review actions.

6557f04

Review actions.

4e9bbe0

Merge branch 'master' into cmduia2-textinfos

b12cbc9

michaelDCurran requested changes Jun 5, 2019

View reviewed changes

Fix forward movement across lines.

e4e96ef

UIAWinConsole support: Improve logic of moving previous word to no lo…

d00a15b

…nger jump over the last word on the previous line.

codeofdusk added 2 commits June 5, 2019 12:22

Merge remote-tracking branch 'origin/pr9647_fixMovePreviousWord' into…

1df93fa

… cmduia2-textinfos

Style.

9fef531

michaelDCurran approved these changes Jun 5, 2019

View reviewed changes

michaelDCurran merged commit 06539e6 into nvaccess:master Jun 5, 2019

nvaccessAuto added this to the 2019.3 milestone Jun 5, 2019

codeofdusk mentioned this pull request Jun 5, 2019

Preliminary support for UI Automation in Windows Console #9614

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI Automation in Windows Console: limit blank lines in review and initial word movement support #9647

UI Automation in Windows Console: limit blank lines in review and initial word movement support #9647

codeofdusk commented May 31, 2019

codeofdusk commented May 31, 2019

michaelDCurran Jun 2, 2019

michaelDCurran Jun 2, 2019

michaelDCurran Jun 2, 2019

michaelDCurran Jun 2, 2019

michaelDCurran Jun 2, 2019

michaelDCurran Jun 2, 2019

michaelDCurran Jun 2, 2019

michaelDCurran Jun 3, 2019

michaelDCurran Jun 3, 2019

michaelDCurran Jun 3, 2019

michaelDCurran commented Jun 4, 2019

michaelDCurran commented Jun 4, 2019

codeofdusk commented Jun 4, 2019

LeonarddeR Jun 4, 2019

LeonarddeR Jun 4, 2019

LeonarddeR Jun 4, 2019

codeofdusk Jun 4, 2019

LeonarddeR Jun 4, 2019

LeonarddeR Jun 4, 2019

LeonarddeR Jun 4, 2019

michaelDCurran left a comment

codeofdusk commented Jun 5, 2019

michaelDCurran commented Jun 5, 2019 •

edited

michaelDCurran commented Jun 5, 2019

michaelDCurran commented Jun 5, 2019 •

edited

codeofdusk commented Jun 5, 2019

		@@ -140,8 +151,8 @@ def _getCurrentWord(self):
		lineInfo = self.copy()

	if not self._isCaret:
	if self.basePosition != textInfos.POSITION_CARET:

-			if visiRanges.length > 0:
+			visibleRangesLength = visiRanges.Length
+			if visibleRangesLength > 0:

-		while True:
+		while charInfo.compareEndPoints(
+			lineInfo,
+			"startToEnd"
+		) <= 0:

UI Automation in Windows Console: limit blank lines in review and initial word movement support #9647

UI Automation in Windows Console: limit blank lines in review and initial word movement support #9647

Conversation

codeofdusk commented May 31, 2019

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

codeofdusk commented May 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelDCurran commented Jun 4, 2019

michaelDCurran commented Jun 4, 2019

codeofdusk commented Jun 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelDCurran left a comment

Choose a reason for hiding this comment

codeofdusk commented Jun 5, 2019

michaelDCurran commented Jun 5, 2019 • edited

michaelDCurran commented Jun 5, 2019

michaelDCurran commented Jun 5, 2019 • edited

codeofdusk commented Jun 5, 2019

michaelDCurran commented Jun 5, 2019 •

edited

michaelDCurran commented Jun 5, 2019 •

edited