Avoid unnecessary sleep when handling caret movement to improve responsiveness.

[jcsteh]

Link to issue number:

None.

Summary of the issue:

NVDA frequently takes 30 to 45 ms to respond to cursor movement (arrow keys, etc.) in a text box. This doesn't seem like much, but anything we can do to improve responsiveness here makes for a better experience for the user.

Description of user facing changes

NVDA responds faster when moving the cursor in edit controls.

Description of development approach

1. Don't sleep or yield in KeyboardInputGesture.send. We sleep for 10 ms, which is at the upper end of acceptable audio latency for "instantaneous". Furthermore, the Windows timer resolution is only accurate to about 15.6 ms most of the time.
   This sleep was apparently added to ensure NVDA's keyboard hook ignored the injected keys. In my testing, removing this didn't seem to have any negative impact. If this does turn out to be a problem, we could watch for the last injected key in the keyboard event hook and fire a Windows event when it arrives. This will be faster to wake than a 10-16 ms sleep.
2. When waiting for caret movement, spin the first few tries rather than sleeping. Again, we might sleep longer than we need to and sometimes apps respond much faster than we previously allowed for. Spinning for a very short while improves responsiveness without excessive resource usage.
3. The caret movement waiting code was also refactored a bit to better calculate the elapsed time. We were previously relying on multiplying the sleep amount, which was inaccurate because sleep is inaccurate. For example, if we retried 3 times with a 10 ms retry interval, we would report 30 ms elapsed. However, each 10 ms could have been up to 16 ms in reality, resulting an actual elapsed time of 48 ms. It also didn't account for the time it took to query the control. Now we use the system clock to measure the elapsed time.
4. Increase the use events timeout from 10 ms to 60 ms. As noted in 3), we were previously multiplying the sleep amount, which meant our supposed elapsed time was actually less than what had truly elapsed. This just updates the timeout to reflect reality. Without this, backspacing occasionally failed in UIA Notepad and the search box in File Explorer.

Note that this does alter some class variables in EditableText. However, these were private (underscore prefixed), so I don't think this should be considered API breaking.

Testing strategy:

I used the "time since input" logging to look at the time taken to speak when moving by character in the Run dialog and a simple text box in Firefox. Before this change, this is usually somewhere between 30 and 50 ms. After the change, this is usually somewhere between 20 and 40 ms, though I've seen it go to 10 ms or lower. This is quite variable, so it's hard to be scientific about this. Combined with #14701, this definitely feels snappier to me.

Known issues with pull request:

None.

Change log entries:

Bug fixes
NVDA responds faster when moving the cursor in edit controls.

Code Review Checklist:

• Pull Request description:
  • description is up to date
  • change log entries
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• API is compatible with existing add-ons.
• Documentation:
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English
• Security precautions taken.

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests (tags: installer NVDA).
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/1gl4aflkcvf7i5j9/artifacts/output/nvda_snapshot_pr14708-27824,d136d6d9.exe
• CI timing (mins):
  INIT 0.0,
  INSTALL_START 0.9,
  INSTALL_END 0.9,
  BUILD_START 0.0,
  BUILD_END 23.7,
  TESTSETUP_START 0.0,
  TESTSETUP_END 0.3,
  TEST_START 0.0,
  TEST_END 22.9,
  FINISH_END 0.1

See test results for failed build of commit d136d6d9ba

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: Unit tests.
• PASS: Lint check.
• FAIL: System tests (tags: installer NVDA). See test results for more information.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/fcvckrfskwearyus/artifacts/output/nvda_snapshot_pr14708-27826,c3a719ac.exe
• CI timing (mins):
  INIT 0.0,
  INSTALL_START 0.8,
  INSTALL_END 0.9,
  BUILD_START 0.0,
  BUILD_END 24.7,
  TESTSETUP_START 0.0,
  TESTSETUP_END 0.3,
  TEST_START 0.0,
  TEST_END 23.1,
  FINISH_END 0.1

See test results for failed build of commit c3a719aca5

[michaelDCurran]

Looks good to me.
However, apparently it is common for some people to hold down commands like downArrow or control+downArrow in large documents to skim the structure or something. We had had reports in the past of freezes or keys being locked down when this code is touched. I think you removed the yield? Have you done any testing in scenarios like these?

[cary-rowen]

@michaelDCurran
I tested using the steps in #13334. Still reproducible, but at least this PR didn't make it any worse.

[jcsteh]

I have tested quite a bit with holding down the downArrow key. I haven't tested with control+downArrow though. I'll do that. It sounds like I should also test with control+shift+downArrow based on #11478.

[jcsteh]

I've just held down control+downArrow and control+shift+downArrow for a long time in a pretty long document and didn't encounter any freeze or anything like that. Still, I'll watch out for it while using it in daily work.

[Adriani90]

Might also improve the behavior described in #13484.

[jcsteh]

I doubt it. This PR is about improving the response time between pressing a key and NVDA checking whether the cursor has moved. That issue sounds like it's more about NVDA not being able to get correct info from the editor for somereason. If anything, that issue probably requires a slower response time.

[amirmahdifard]

thank you for this, i also have some delay with pressing keys and getting responce from nvda sometimes, i hope this will fix that problem: thanks?

[codeofdusk]

This PR makes tracking of deleted characters in UIA Word much less reliable:

1. Open a new email message in Outlook.
2. Tab to the body.
3. Enter "abc", then press backspace. Observe that the deletion of anything from zero to all three characters are reported.
4. Repeat steps 2 and 3. Observe possibly different results.

[jcsteh]

Is this with the latest commit in this branch? If so, we might need to raise _useEvents_maxTimeoutSec even further to 0.03. I already raised it to 0.02, but it might not be enough. I explained why this is necessary in the edited PR description:

4. Increase the use events timeout slightly from 10 ms to 20 ms. As noted in 3), we were previously multiplying the sleep amount, which meant our supposed elapsed time was actually less than what had truly elapsed. This just updates the timeout to reflect reality. Without this, backspacing occasionally failed in UIA Notepad.

[jcsteh]

To explain this further, we previously had a retry interval of 0.01 and we just kept multiplying that by the number of retries to calculate the elapsed time; e.g. 0.01, 0.02, 0.03, etc. The problem is that because of the Windows timer resolution, this could be quite inaccurate. If each sleep took 16 ms instead of 10 ms, the elapsed time for 3 tries would be 0.048, 18 ms longer than we calculated.

[codeofdusk]

Is this with the latest commit in this branch?

Yes it was. Initial testing with 0.03 looks promising, but I'll run with it for a while to be sure.

[codeofdusk]

Thanks for the update @jcsteh – so far so good with 0.03!

[codeofdusk]

@jcsteh Found aanother bug with the latest commit in this branch:

1. Open a folder in Windows Explorer.
2. Tab to the search field.
3. enter "abc".
4. Press backspace three times to delete all three characters.

Expected: NVDA reports "c", "b", "a".

Actual: NVDA reports "c", "b".

[jcsteh]

I couldn't reproduce the search bug exactly as you described. However, I did see intermittent failures when backspacing.

To address this, I've had to raise the event timeout even higher to 60 ms. It looks like that control responds very slowly, both in firing events and in processing queries. Not only did our elapsed time previously not account for sleep inaccuracy, it also didn't account for query time.

I'm actually not quite sure why we have that event timeout at all. I looked at the various pull requests which introduced the caret event changes for UIA, but I don't see a justification for the event timeout.

[michaelDCurran]

@codeofdusk is the explorer search field working okay for you now with the latest fix from @jcsteh ?

[codeofdusk]

@michaelDCurran Yes, it does appear to work now!