Fix #9786 #9787
Fix #9786 #9787
Conversation
| _caretMovementTimeoutMultiplier = 1 | ||
| #: The caret movement timeout is multiplied by this value (useful for | ||
| #: controls where the caret is slow to move). | ||
| #: Set to a number grater than 0 to enable caret event handling |
There was a problem hiding this comment.
I had to look into the changes in #9773) to understand this pr correctly.
I'm a bit worried about the current approach:
-
I haven't yet read through UI Automation in Windows Console: work around Microsoft bugs on Windows 10 version 1903 and improve caret movement #9773 completely, but I'm missing the rationale of why EditableText._hasCaretMoved has changed to check caret events. It is probably way faster than checking bookmarks?
-
This pr ties the caretMovementTimeoutMultiplier to the logic that checks for pending caret events. I really prefer the multiplier to be just a multiplier, e.g. 1 should be the default, setting it to 0 should not disable multiplication.
-
Rather than this, I think the bug must be found/fixed in EditableText._hasCaretMoved. The while loop now behaves as follows:
- It returns False isScriptWaiting
- It processes pending events
- It returns True on a focus event
- New code: it returns True for a caret event, thereby passing along the textInfo for the current caret, which is probably lagging behind (see below)
- After that, it also creates textInfo for the caret, and there is a comment that reads:
Caret events are unreliable in some controlsI think this is exactly what's happening in Chromium. The caret events are unreliable, i.e. there is a caret event, but when fetching the textInfo for the caret, the textInfo still fetches the previous caret position.
I think we should consider reverting #9773 for now and put some more thought into a way to handle these issues more reliably.
Link to issue number:
Fixes #9786 (follow-up of #9773). Related to #9614.
Summary of the issue:
NVDA responds to
caretevents from all controls that support them, but some implementations incorrectly fire them.Description of how this pull request fixes the issue:
We now only enable
caretevent handling if a control has a_caretMovementTimeoutMultipliergreater than 0.Testing performed:
Tested caret movement in the Google search field on Chrome 76.0.3809.36 before and after this change, and observed that it restores caret movement support.
Known issues with pull request:
None.
Change log entry:
None.