winConsoleUIATextInfo: use rangeFromPoint instead of broken getVisibleRanges #10559
Conversation
…is very broken, use IUIAutomationTextPattern::rangeFromPoint for the top left and bottom right of the console window.
|
@codeofdusk I'd appreciate your help in testing this where you can. I was originally against this approach, but since getvisibleRanges is very broken, I don't think we have a choice anymore. |
After preliminary testing, I found that:
|
|
I'm guessing that perhaps when review stops working when the console is
flooded, this is when in the existing code new text announced was
invalid, and review also was broken? Or are you saying that this pr is
actually worse?
I think I can definitly address the speed issue by putting back
_getTextLines that calls IUIAutomationTextRange::getText directly. The
base _getTextLines breaks text by unit line which is rather slow for
UIA. Rather as this UIA implementation contains linefeed characters, we
can just do a python string splitlines.
|
This PR is worse. Previously, autoread stopped completely but review still worked. Also, a |
…at just uses Python string splitlines.
|
PR introduces Flake8 errors 😲 See test results for Failed build of commit 7518e2c85c |
|
It seems that the |
|
Also, it seems that your |
…fix review bounds.
|
PR introduces Flake8 errors 😲 See test results for Failed build of commit 46b0888fd9 |
|
@codeofdusk I think I have improved POSITION_FIRST / POSITION_LAST so that now review top and review bottom behave after a lot of text has scrolled up the screen. |
Things are a lot better, but: STR:
Expected: the last word of the prompt is read out. |
Nevermind, I think that was confusion on my part (as now consoles have blank lines at the end per known issues). The confusing part is that word and line movement have different bounds (i.e. I can access some parts of the console with word review that I can't with line review, and vice versa). |
…t directly with IUIAutomationTextRange::getText as WinConsoleUIATextInfo.text has been changed to return a space for empty ranges.
|
It seems moving by word was a little broken after your pr was merged to
return a space from the text property. I have worked around this by
calling IUIAutomationTextRange::getText directly from the word and line
offset calculation functions.
|
|
Also, I re-tested the console flooding case. This PR doesn't regress behavior. |
|
@michaelDCurran When do you plan to merge this PR? |
|
Branch policy dictates that I require a review approval from a
"collaborator" such as @LeonarddeR before I can merge.
|
| # Therefore use the bottom left, then move the to last character on that line. | ||
| tempInfo = self.__class__(obj, obj.location.bottomLeft) | ||
| tempInfo.expand(textInfos.UNIT_LINE) | ||
| UIATextInfo.move(tempInfo, textInfos.UNIT_CHARACTER, -1, endPoint="end") |
There was a problem hiding this comment.
What happens if we don't do this? We already expanded to line, so the endpoint should be the end of the line already?
| visiLength = visiRanges.length | ||
| if visiLength > 0: | ||
| oldRange = self._rangeObj.clone() | ||
| boundingInfo = self.obj.makeTextInfo(textInfos.POSITION_ALL) |
There was a problem hiding this comment.
Consistency with other code
| boundingInfo = self.obj.makeTextInfo(textInfos.POSITION_ALL) | |
| boundingInfo = self.__class__(obj, textInfos.POSITION_ALL) |
| oldRange | ||
| and isTextRangeOffscreen(self._rangeObj, visiRanges) | ||
| and not isTextRangeOffscreen(oldRange, visiRanges) | ||
| oldInfo |
There was a problem hiding this comment.
Could you add a small comment that explains what this if statement tries to do? It is hard to read at first glance.
|
@LeonarddeR I have improved comments as requested. However, I did not take your suggestion of changing the makeTextInfo call into a direct WinConsoleUIATextInfo construction as this is not needed in the move method -- it can stay high-level. The only reason I did it in WinConsoleUIATextInfo's init method was because I was passing in _rangeObj which makeTextInfo does not allow. |
Co-Authored-By: Leonard de Ruijter <[email protected]>
Link to issue number:
Fixes #10191
Fixes #10119
Summary of the issue:
In Windows consoles with UIA, NVDA currently makes use of IUIAutomationTextPattern::getVisibleRanges to find the bounds of what is visibly on screen. This is both for bounding the review cursor, and for collecting the on-screen text for diffing, in order to speak what has changed.
However, IUIAutomationTextPattern:getVisibleRanges can eventually start giving back junk text ranges that are positioned way above, and outside of the real visible bounds. This seems to start happening once a great deal of text has been scrolled up the screen.
This not only causes the review cursor to act strangely or get stuck when moving to the top or bottom, but it causes very old and invalid text to be spoken when things change on screen.
Description of how this pull request fixes the issue:
Instead of getVisibleRanges, use IUIAutomationTextPattern::rangeFromPoint at the top left and bottom right of the console window.
Testing performed:
Tested in both wsl (Ubuntu) and cmd.
Tested when older alpha builds of NVDA got into a state where the console no longer made sence with getVisibleRanges; this approach seemed to give back correct text and the review cursor no longer got stuck.
Known issues with pull request:
Like the older legacy winConsole support, if moving to the bottom with the review cursor and then going up by line, if there are blank lines in the console, these are not skipped. this is a little annoying for efficiency, but it is the same behaviour we originally had for many years, and blank lines are certainly better than invalid text or getting stuck.
Change log entry:
None.