I6060 add emulated kb gesture support

[feerrenrut]

Link to issue number:

supersedes #7784
Closes #6060.

Summary of the issue:

Emulated system keyboard key scripts can only be added in gestures.ini or by updating a gesture map programmatically.

For example, if a user wants to map 'numpad home' act like the regular 'home' key, they have to modify the gestures.ini file manually to include kb:home = kb:numpad7.

Description of how this pull request fixes the issue:

This PR takes the work done in #6060 and makes it compatible with the virtual tree. More than that, it splits out the input gestures dialog, and re-works much of the internal state modelling. Attempting to split responsibilities for managing gesture state from presentation.

Filtering no longer needs a separate thread, by ensuring all redrawing / expanding happens in a freeze / thaw block. This simplifies the logic for filtering.

The tree no longer needs to know how to supply the count or text for pending gestures / emulated gestures. This is handled by the view model.

Testing performed:

• Added / removed emulated gestures.
  • Ensured these persisted when saving (as long as a gesture is assigned), and discarded when cancelling
• Added / removed regular gestures.
  • Ensured these persisted when saving, and discarded when cancelling
• Reset gestures.
• Tested filtering

Known issues with pull request:

This is a lot of changes, and will break compatibility for add-ons that modify the behavior of the Input gestures dialog. I don't know of any addons that rely on this.

Change log entry:

New features:

    - New emulated system keyboard keys can be added from NVDA's Input gestures dialog. (#6060)
        - To do this, press the add button after you selected the Emulated system keyboard keys category.

[feerrenrut]

I think scripts would be right, as these are all scripts (i.e. methods
with the script_ prefix on objects). The word command is pretty
confusing indeed.

I have removed the term command. Though it's worth noting that with the introduction of emulated gestures, which masquerade as scripts, its not quite true that everything at that level is a script. There may be value in having a generic concept (such as 'command') for an action that occurs as a result of user input. Many other applications call these commands. After all, many of these live in the globalCommands file.

[feerrenrut]

@CyrilleB79

1- Inconsistency of tree element label between the moment when the emulated key is added and a future reopening of the gesture dialog:

I have fixed this issue.

2- Issue in key emulation:

I won't try to fix this in this PR. But I'll attempt to write something in the user guide shortly.

3. Enable remove button even when the emulated key has associated gestures:

I wont do this now. I don't think it would be common to have many gestures mapped to a single emulated gesture, and I don't want to go down the rabbit hole on the UX for removing these items. I think what we have now is acceptable.

[feerrenrut]

@Qchristensen Could you please review the changes to the user guide?

[feerrenrut]

But I also have items such as: Emulate key press: Virtually toggles the alt key to emulate a keyboard shortcut with braille input

@CyrilleB79 That issue should now be fixed.

[Qchristensen]

I'm just reviewing the user guide change, which looks fine - I did have to read the note example twice to comprehend it, however it does make sense and I can't think of a simpler way of expressing it. Good work everyone on what looks like a very large piece of work all up!

[michaelDCurran]

I had experienced a crash with this pr and had commented so, but have now removed the comment as it was my mistake for not recompiling nvdaHelper. Sorry for the noise.

[michaelDCurran]

In general this all looks good to me. However, there is a bug stopping many gestures with modifiers from being correctly emulated (E.g. NVDA+f).

[michaelDCurran]

Everything looks good to me a part from changing normalizedIdentifiers to identifiers where I have commented in this review.

[michaelDCurran]

Ibelieve this should actually be identifiers, not normalizedIdentifiers on this line.
The reason for this is that eventually this string will be passed to KeyboardInputGesture.fromName, but fromName cannot currently cope with a normalized identifier (where the main key may not necessarily appear after the modifiers).
Take the example of trying to emulate NvDA+f:
identifiers is kb:nvda+f but normalizedIdentifiers is kb:f+nvda (as it has been reordered in a determinate way for internal matching).
But, if you pass "f+nvda" to KeyboardInputGesture.fromName, you get back a rather broken gesture with normalizedIdentifier of "kb:+nvda".
You could argue this is a bug in KeyboardInputGesture.fromName, but I'm not so sure as fromName really should only handle a user-acceptable representation, and not any kind of internal representation.
Changing normalizedIdentifiers to identifiers on this line then will correctly emulate kb:nvda+f, correctly matching and executing the reportFormatting script, or if that was not mapped, correctly sending it on to the OS.
Note that this also affects emulating shift+downArrow. In deed anything that gets reordered in normalizedIdentifiers.

[feerrenrut]

The thing that worries me about using a non-normalized name as the key for emulations is the potential for bugs based due to localization / or different orderings of the same gesture. But maybe identifiers is still acceptable for this purpose?

[feerrenrut]

In particular the docs for inputCore.InputGesture._get_identifiers

If C{id} contains multiple chunks separated by a + sign, they are considered to be ordered arbitrarily
and may be reordered when normalized.
Normalization also ensures that the entire identifier is lower case.
For example, NVDA+control+f1 and control+nvda+f1 will match when normalized.
See L{normalizeGestureIdentifier} for more details.

[michaelDCurran]

Only worrying about KeyboardInputGesture (as we only emulate keyboard input) we just need a value that we know will work with KeyboardInputGesture.fromName. It will never be used anywhere else. I can tell you that KeyboardInputGesture.identifiers predictably produces a value that is modifiers+mainKey, which is what fromName expects. Where as normalizeIdentifiers is re-ordered for internal matching of gestures. Remember here the goal is to make a new Gesture, not match against one.
If you really don't want to use identifiers here, then you'll need to change KeyboardInputGesture.fromName to allow the main key to appear before one or more modifiers. Though I feel that kind of change is actually quite a bit more risky as getting it wrong could affect much more in NVDA.