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

V2 of UI Automation in Windows Console: work around Microsoft bugs on Windows 10 version 1903 and improve caret movement #9802

Merged
merged 3 commits into from
Jun 24, 2019

Conversation

codeofdusk
Copy link
Contributor

Link to issue number:

Closes #9632 and #9649. Builds on #9614. Incorporates the __ne__ method from #5991 for UIA console textInfos.

Summary of the issue:

Currently:

  • On Windows 10 1903 and later, the collapse method on consoleUIATextInfo is broken, resulting in several caret and selection bugs.
  • In UIA consoles, caret move detection is wildly inaccurate.

Description of how this pull request fixes the issue:

  • Re-implements consoleUIATextInfo.collapse and consoleUIATextInfo._get_isCollapsed as suggested by @michaelDCurran.
  • Adds a __ne__ method to consoleUIATextInfo to allow for better caret move detection.
  • Adds a new _caretMovementTimeoutMultiplier variable to editable text objects, and sets it to 1.5 for consoles (caret movement over ssh can be slowe).

Note: unlike #9773, this PR does not depend on caret events (which caused #9786).

Testing performed:

Tested the console on Windows 10 1903. Caret movement, selection, and backspace reporting are functional, even over ssh.

Known issues with pull request:

  • Occasionally in UIA consoles, the last character of the prompt is still read out when pressing backspace, especially if quickly deleting text (maybe fixed with these last changes, as I haven't been able to reproduce it since).

Change log entry:

None.

@codeofdusk
Copy link
Contributor Author

Copy link
Collaborator

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

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

I've hade some nights sleep about this.

While it caused issues, I really liked the idea that editableText._hasCaretMoved processed caret events. How about something like this (would be good to know @michaelDCurran's pinion before starting on this).

  1. Below the Caret events are unreliable in some controls. comment, add something like this:
if self.detectCaretMovementUsingEvents and eventHandler.isPendingEvents("caret"):
	log.debug("Caret move detected using event. Elapsed: %d ms" % elapsed)
	return (True, newInfo)

Then, detectCaretMovementUsingEvents should be False by default, and only set to True for implementations where we are really sure that the textInfo doesn't lag behind.

@@ -187,6 +204,10 @@ def _getWordOffsetsInThisLine(self, offset, lineInfo):
min(end.value, max(1, len(lineText) - 2))
)

def __ne__(self,other):
"""Support more accurate caret move detection."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does ne relate to caret move detection?

Copy link
Member

Choose a reason for hiding this comment

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

Can you explain exactly what happens if you don't implement this method?

Copy link
Member

Choose a reason for hiding this comment

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

__ne__ is necessary as UIATextInfo has overridden __eq__```. So we want ```__ne__``` to return the opposite. This should really actually be on UIATextInfo itself, but this has caused strange bugs in the past, espcially in Edge. We are not sure why IUIAutomationTextRange.compare does not work correctly for some implementations. In consoles however, it does work okay. Note that in Python3 it will no longer be neceesary to provide a ```__ne__``` here as Python3 automatically implements ```__ne__ as the negative of __eq__.

@@ -187,6 +204,10 @@ def _getWordOffsetsInThisLine(self, offset, lineInfo):
min(end.value, max(1, len(lineText) - 2))
)

def __ne__(self,other):
"""Support more accurate caret move detection."""
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain exactly what happens if you don't implement this method?

@michaelDCurran
Copy link
Member

I've hade some nights sleep about this.

While it caused issues, I really liked the idea that editableText._hasCaretMoved processed caret events. How about something like this (would be good to know @michaelDCurran's pinion before starting on this).
I think we can leave this change for another pr. That would be lest risky.

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.

UIA in Windows Console: Space Character is not Announced when Deleting Characters
4 participants