Skip to content

Conversation

@mwcampbell
Copy link
Contributor

This allows screen readers and other ATs to provide the expected feedback when editing text on Windows.

The easiest way to try it out with a working application is to try one of the examples in the corresponding egui branch.

The primary missing feature is support for rich text, including text formatting information.

This ought to have tests, but I figured I'd get the implementation into code review.

Copy link
Member

@DataTriny DataTriny left a comment

Choose a reason for hiding this comment

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

First pass: egui examples work flawlessly!
We really need an extensive test suite for this one as I'm sure I missed some edge cases.
Outstanding work!

@mwcampbell
Copy link
Contributor Author

The new test suite is not yet comprehensive, but I think it covers enough essential cases that we can consider it good enough for now.

Copy link
Member

@DataTriny DataTriny left a comment

Choose a reason for hiding this comment

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

I am very happy with these changes. I don't see a newline character at the end of text fields anymore.

You still haven't addressed my concern regarding cross-platform newline parsing for paragraphs. If AccessKit should only support \n, then it must at least be documented somewhere.

There is a weird behavior when walking to the next word as the caret is placed at the end of the current word (so usually at the trailing space). I suppose this is a bug in egui, mentioning it here just in case.

Finally, I am not able to use my Braille display routing buttons to move the cursor. When I click above a character, it is only spoken by NVDA but the caret position remains unchanged. I can't see any error on NVDA's log. For reference, I have attached logs demonstrating what is supposed to happen when using the routing keys on a UIA text field (like in the hello_world example, the value is "Arthur", and I clicked on "r").

NVDA logs.txt

@mwcampbell
Copy link
Contributor Author

@DataTriny The fact that line breaks are expected to be either CRLF or LF is actually mentioned in the documentation for Node::character_lengths.

egui's behavior for moving by word is indeed not what Windows users and screen readers expect. I'll see if I can get it fixed in egui.

The reason your cursor routing keys aren't working yet is that I forgot to implement the SetTextSelection action in my egui branch. I'll work on that now.

@DataTriny
Copy link
Member

@mwcampbell Oh sorry, I missed the note on line breaks on the character_lengths documentation.

Then I'll wait to test the SetTextSelection implementation, and we should be able to merge this quickly!

@mwcampbell
Copy link
Contributor Author

@DataTriny Please test the cursor routing keys with the latest commit on the accesskit-text branch of my egui repo. I had already dispatched the action correctly on the AccessKit site; I just needed to implement it in egui.

@DataTriny
Copy link
Member

@mwcampbell The routing keys work now!

However, there is another issue I didn't notice before when hovering the mouse over a text field, but it is not related to your latest change. There is no visible bug, but NVDA complains and spits this kind of error whenever the mouse moves:

ERROR - eventHandler.executeEvent (17:13:47.552) - MainThread (20344):
error executing event: mouseMove on <NVDAObjects.Dynamic_EditableTextWithAutoSelectDetectionUIA object at 0x06876E10> with extra args of {'x': 213, 'y': 121}
Traceback (most recent call last):
  File "eventHandler.pyc", line 301, in executeEvent
  File "eventHandler.pyc", line 102, in __init__
  File "eventHandler.pyc", line 111, in next
  File "NVDAObjects\__init__.pyc", line 1158, in event_mouseMove
  File "documentBase.pyc", line 61, in makeTextInfo
  File "NVDAObjects\UIA\__init__.pyc", line 463, in __init__
  File "monkeyPatches\comtypesMonkeyPatches.pyc", line 32, in __call__
_ctypes.COMError: (-2147467263, 'Not implemented', (None, None, None, 0, None))

@mwcampbell
Copy link
Contributor Author

@DataTriny Good catch. I really should implement hit-testing within text, via the ITextProvider::RangeFromPoint method, to complete this feature.

@mwcampbell
Copy link
Contributor Author

@DataTriny OK, NVDA and other ATs should now correctly respond to mouse movement within the text edit. Implementing this turned out to be complicated, so I wrote what I think are thorough tests for this part.

@mwcampbell
Copy link
Contributor Author

@DataTriny I forgot to mention, you should update to the latest commit on the egui accesskit-text branch, so you get the latest Cargo.lock.

…ed when we have lines made up of multiple text boxes.
Copy link
Member

@DataTriny DataTriny left a comment

Choose a reason for hiding this comment

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

Outstanding work! Let's ship this!

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.

3 participants