When handling caret movement, fetch the new caret info after checking for a pending caret event.#16711
Conversation
WalkthroughRecent changes to the project include updates to improve the reliability of caret movement detection in editable text areas. This is achieved by adding a check for elapsed time and specific event types before fetching caret information. Additionally, documentation has been updated to reflect that NVDA now correctly reports characters, words, and other navigation elements when using cursor keys in text boxes within UI Automation-based applications. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
… for a pending caret event.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
I wonder whether we should re-investigate all cases where |
|
It turns out that I can reproduce this in text boxes in web content in Edge when NVDA is using UIA for Edge. |
|
In particular, I'd like to play with this branch in UIA terminals. I disabled caret events there because when quickly backspacing, the last character of the prompt was sometimes eroneously read. Leaving this here and will do so soon |
|
@jcsteh - does this mean this PR should not be merged and a different approach is found? |
|
Sorry for the lack of clarity. I meant that the original issue (without this PR) can be reproduced in Edge + UIA. This PR should fix the problem for both Firefox and Edge when using UIA. I thought this worth noting because while UIA in Firefox is not likely to be used "in the real world", NVDA does have explicit support for UIA in Edge and that is the only option for certain use cases, so that increases the user impact of this bug from theoretical to practical. |
Partially reverts #14888, #15838 Summary of the issue: In #14888, XamlEditableText was added to ensure that UIA events wouldn't be used to determine caret changes, as the hypothesis was that they were fired too early. In #15838, I expanded this workaround for WPF (Visual Studio) text controls. It turns out @jcsteh found the actual cause of these issues and fixed them in #16711, allowing us to rely on events again. Description of user facing changes None, though caret movement would possibly be detected a little bit faster in XAML and WPF controls. Description of development approach Mostly reverts.
Follow-up of #16711, #16817. Summary of the issue: UIA caret events in Windows Terminal were unreliable before #16711. In particular, when rapidly pressing backspace, sometimes the last character of the prompt would be erroneously read. Description of how this pull request fixes the issue: Re-enable caret events in Windows Terminal specifically. Note that caret events are still broken in this way in Conhost and are therefore left disabled.
|
@jcsteh Can this fix also be applied to notepad please? The same issue occurs in notepad, too. |
|
@burakyuksek Notepad is a UIA app. |
|
@codeofdusk I'm aware, however the issue still occurs. |
|
This PR should apply to all UIA implementations, so if this is still happening in Notepad for you, that would suggest it is a different issue. A couple of possibilities:
Otherwise, I have no idea. |
|
@jcsteh Hello, |
5 participants
Link to issue number:
None.
Summary of the issue:
In Firefox's in-progress UIA implementation, moving with the cursor keys in editable text boxes sometimes reports the character at the old position of the caret instead of the new position of the caret. For example, on the line:
abcdef
with the cursor positioned on "a", pressing right arrow several times might read a, b, c instead of b, c, d.
I don't anticipate that NVDA will ever move to using UIA in Firefox. Nevertheless, this is an NVDA bug which might impact other things.
Description of user facing changes
When navigating with the cursor keys in text boxes in applications which use UI Automation, NVDA no longer sometimes reports the wrong character, word, etc.
Description of development approach
Previously,
EditableText._hasCaretMovedfetched the caret before checking for a pending caret event. It then returned this fetched caret. The following race condition could occur:This can't happen with other APIs because they all queue events on the main thread, which can't be preempted during
_hasCaretMoved. However, UIA queues events from a background thread.Testing strategy:
I tested caret navigation in various apps: normal Firefox, Edge, Windows Run dialog, Notepad++, Windows Settings, Windows Start Menu. Everything worked as expected.
I tested caret navigation in Firefox's in-progress UIA implementation. After this change, everything worked as expected.
Known issues with pull request:
None.
Code Review Checklist:
Summary by CodeRabbit