Skip to content

Windows 10/11 Calculator: refactor results announcement with UIA notification event#13625

Merged
seanbudd merged 8 commits intonvaccess:masterfrom
josephsl:i13383calculatorResultsNotificationRefactor
Apr 27, 2022
Merged

Windows 10/11 Calculator: refactor results announcement with UIA notification event#13625
seanbudd merged 8 commits intonvaccess:masterfrom
josephsl:i13383calculatorResultsNotificationRefactor

Conversation

@josephsl
Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #13383

Summary of the issue:

With the initial version of Windows 10/11 Calculator app module, only a select number of keys were defined to announce results. However it turns out there are commands that should be announced such as equals (=) and scientific calculator commands such as s (sign).

Description of how this pull request fixes the issue:

Refactor UIA notification event handler with use of a new flag specifying gestures that should not announce results. If this flag is on (for now when number keys are pressed), NVDA will not announce results. Also, existing result announcement commands will not announce results in Calculator version 10.1908 as notification event will announce result.

Testing strategy:

Manual testing (enter multiple calculations in Calculator, including from standard, scientific, and unit converter modes).

Known issues with pull request:

On newer Calculator releases, NVDA will no longer announce calculator display content if result keys are pressed multiple times, but this aligns better with Narrator behavior.

Change log entries:

Bug fixes or changes:
In Windows 10 and 11 Calculator version 10.1908 and later, NVDA will announce results when more commands are pressed such as commands from scientific mode. (#13386

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

… the base app module for Windows 11 Calculator (calculatorapp) as well.
In recent Calculator releases (prominently from 10.1908 but exact version unknown), UIA notification event is used to anounce results. Back when Calculator used name change and live region events, it was necessary to define result announcement keys such as Enter key that will cause NVDA to announce results. With the introduction of UIA notification event, the new event is used to announce results. This resulted in NVDA announcing repeats when numbers were entered, more so if speak typed characters is on.
In the initial version of Windows 10 Calculator app module (2020) and subsequently amended in 2021, only a select number of keyboard commands (gestures) were defined to announce results. This however led to NVDA not announcing Calculator display when additional result keys such as equals (=) and scientific calculator commands such as S for sine were pressed, requiring users to perform read current line/focused object to obtain results. To resolve this and to respond to UIA notification events properly while keeping the number entry bug fix intact, a new flag is used to preventNVDA from handling notification event if number row and/or numpad keys with num lock on are pressed. This flag is set if number keys are indeed entered and cleared from notification event (next set of commits). In addition, existing result announcement commands wil be tweaked to do nothing if using later Calculator releases since notification event is used to announce resuts anyway (this means verbosity changes such as NVDA not announcing display content if Escape key is pressed multiple times, for example). Note that the new flag (no result gestures) should not be confused with existing 'should announce results' flag as the existing flag is used to handle old Calculator releases with no support for notification event.
…estures are pressed. Re nvaccess#13383.

Do not handle UIA notification event if no result gestures flag is set in order to avoid repetitions. As the flag is set from a script (see next commit), have a copy of this flag before clearing the flag from notification event. The event handler copy of the no results flag will be used later when handling 'DisplayUpdated' activity Id (results display), and if set, return from the event handler.
… Re nvaccess#13383.

In addition to notification event handler tweak, add a script that defines gestures for which NVDA should not announce Calculator results. For now number row and numpad keys with num lock turned on are defined specifically to keep no repetition announcement bug fix intact (where NVDA will repeat things if speak typed characters is on).
…ion 10.1908 and later. Re nvaccess#13383.

Bonus: becasue UIA notification event is used to announce results, there is no need for NVDA to check focus in app version 10.1908 and later. Therefore return early from existing results announcement script if running on later Calculator releases (version string comparison will work).
@josephsl josephsl requested a review from a team as a code owner April 22, 2022 00:08
@josephsl josephsl requested a review from michaelDCurran April 22, 2022 00:08
@AppVeyorBot
Copy link
Copy Markdown

See test results for failed build of commit e856651a4c

Comment from Sean Budd (NV Access): better not to use string comparisons (what if comparisons fail for odd Calculator version strings that does not necessarily fit what humans think of newer releases versus what Python gives). Therefore obtain integers for major and minor Calculator releases and compare versions accordingly (integer list comparison).
# #13383: later Calculator releases use UIA notification event to announce results.
# While the exact version that introduced UIA notification event cannot be found easily,
# it is definitely used in version 10.1908 and later.
if self.productVersion >= "10.1908":
Copy link
Copy Markdown
Member

@seanbudd seanbudd Apr 22, 2022

Choose a reason for hiding this comment

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

I would suggest against using a string comparison here, instead split and compare tuples

@josephsl
Copy link
Copy Markdown
Contributor Author

josephsl commented Apr 22, 2022 via email

@AppVeyorBot
Copy link
Copy Markdown

See test results for failed build of commit 343d22e219


# Handle both number row and numpad with num lock on.
@scriptHandler.script(
gestures=[f"kb:{i}" for i in range(10)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unfortunately this does not work on all keyboard localization. On French keyboard, the digits on the number row (on the alpha-numeric keyboard) are accessible in combination with the shift key.
For example, if you press the key above the 'E' (a bit on the right) you get the open parenthesis character. If you press shift in compination with the key above the 'E', you get the digit '4'.

Also to be noted: some characters on the number row are commands of the calculator (parenthesis, minus sign).
The characters that are on the keys 1 to 0 on the number row of the French keyboard are the following:
&é"'(-è_çà

@josephsl
Copy link
Copy Markdown
Contributor Author

josephsl commented Apr 22, 2022 via email

@CyrilleB79
Copy link
Copy Markdown
Contributor

I think that this PR and the calculator appModule are worth to be kept, even in case where it would not handle all use cases since it is already an improvement when typing number on the numeric keypad.

I prefer the following solutions if applicable, from better to worse:

Solution 1:
Define a global overlay class to handle typed character event.
This seems good from a user point of view. Maybe harder to read from a dev point of view.

Solution 3: Add more keys to the one causing UIA notifications to be ignored, i. e. kb:number and kb:shift+number
Issue: This cause the result not to be read even if pressing a symbol on the numeric row, e.g. minus symbol on the French keyboard.

Solution 3: Make the alpha-numeric number key translatable.
Issue: this works only when the keyboard main language matches NVDA's UI language.
Similar assumptions are already present in NVDA's code base, e.g. assuming that Word's UI language matches NVDA's UI one for various Word formatting shortcuts. But this is less than ideal, since one may use two different languages.
As an example for calculator, I am not sure, but I would not be surprised that Canadian people using French NVDA use a keyboard layout which allows to type the number without the shift key.

@josephsl
Copy link
Copy Markdown
Contributor Author

josephsl commented Apr 22, 2022 via email

@CyrilleB79
Copy link
Copy Markdown
Contributor

OK. Thanks for the investigations.

Another option to fix this issue with any type of input device and layout is to report the issue to Microsoft. Since the calculator with Narrator is also very verbose.

@josephsl
Copy link
Copy Markdown
Contributor Author

josephsl commented Apr 22, 2022 via email

@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Apr 27, 2022
@seanbudd seanbudd merged commit 4754bf0 into nvaccess:master Apr 27, 2022
@nvaccessAuto nvaccessAuto added this to the 2022.2 milestone Apr 27, 2022
@josephsl josephsl deleted the i13383calculatorResultsNotificationRefactor branch April 30, 2022 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows 10/11 Calculator: allow NVDA to announce more operations by suppressing only a limited number of keyboard commands

5 participants