Skip to content

Fix for last script count when an unbound gesture is executed between two identical bound gestures#11767

Merged
michaelDCurran merged 5 commits into
nvaccess:masterfrom
CyrilleB79:fixLastScriptCount
Oct 20, 2020
Merged

Fix for last script count when an unbound gesture is executed between two identical bound gestures#11767
michaelDCurran merged 5 commits into
nvaccess:masterfrom
CyrilleB79:fixLastScriptCount

Conversation

@CyrilleB79

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #11388

Summary of the issue:

scriptHandler.getLastScriptRepeatCount is used to detect double/triple press.
However, in the case described in #11388, the user presses 3 keystrokes quickly:

  • NVDA+UpArrow
  • F3
  • NVDA+UpArrow
    The first and the third are bound to a script but not the second.
    Since the second keystroke is not bound to a script, NVDA does not detect it and thus consider that the 2 keystrokes of NVDA+UpArrow is a double key press.

Description of how this pull request fixes the issue:

When a gesture that is not bound to a script is executed, clear the last script ref and count so that the subsequent script is not detected as a repeat.

Testing performed:

Known issues with pull request:

None

Notes for reviewer

I would especially like to have the following points confirmed:

  • where is the best place in executeGesture to call clearLastScript?
  • which variables should be cleared in clearLastScript? Maybe I should also add _lastScriptTime? Or any other private variable located at the beginning of scriptHandler.py (after imports)

Change log entry:

Section: Bug fixes

NVDA no longer detects repeated script gesture inappropriately when executing quickly some sequences of gestures. (#11388)

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 0c4a2c4431

Comment thread source/scriptHandler.py Outdated
Comment thread source/scriptHandler.py
Comment thread source/inputCore.py Outdated
# Clear memorized last script to avoid getLastScriptRepeatCount detect a repeat
# in case an unbound gesture is executed between two identical bound gestures.
if not script:
scriptHandler.clearLastScript()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This needs to be queued to the main thread.
E.g.

queueHandler.queueFunction,queueHandler.eventQueue, scriptHandler.clearLastScript)

Reason: This code is run in the input thread, and there is a chance that those variables will be cleared before a previous script is pulled from the queue in the main thread and executed.

I would also consider moving this code into an else block directly under the call to queueScript at the end of this function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This needs to be queued to the main thread.

Done. Thanks.

I would also consider moving this code into an else block directly under the call to queueScript at the end of this function.

As you suggested, I have moved the call to clearLastScript at the end of the function. Actually, clearing last script variables before handling intercepted command Script case would maybe have caused wrong setting of the variable keeping track of the last executed script.

I have also moved the raise NoInputGestureAction in the else branch since the call to clearLastScript and the raise statement belong to the same execution branch.

@CyrilleB79

Copy link
Copy Markdown
Contributor Author

Thanks @michaelDCurran for this review. I have addressed all comments. Let me know if there is something remaining to be done.

michaelDCurran
michaelDCurran previously approved these changes Oct 20, 2020
@michaelDCurran michaelDCurran merged commit b842b68 into nvaccess:master Oct 20, 2020
@nvaccessAuto nvaccessAuto added this to the 2020.4 milestone Oct 20, 2020
@CyrilleB79 CyrilleB79 deleted the fixLastScriptCount branch October 23, 2020 20:17
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.

NVDA spells the current line even if the user did not perform a double keypress

4 participants