Hello Tristan,

Thank you for your first contribution. As the original author of Calculator app module, I appreciate the effort taken to resolve this issue and update the code logic (and thank you for pointing out how tricky the logic is).

A few general comments:

• I love the honesty when you put "trial and error" in development strategy (yep, even experienced project members write that, so you are not alone). In the future, please provide at least a high-level overview of the proposed code changes (for an example, see Windows 11 emoji panel/navigation menu item: handle UIA elemenet selected event by ignoring it or manipulating NVDA's focus object #16555)..
• Calculator app module contents came from an NVDA add-on called Windows App Essentials (https://github.com/josephsl/wintenapps). When you read the commit log for that project, you will see some emotional content, including frustrations with app accessibility and usability.
• Speaking of code and its origins, the tricky logic part stems from "debates" between Microsoft, apps, yours truly, and NVDA - whenever Microsoft made changes to their apps, I would test them and offer feedback on accessibility for screen reader users, explaining how the code is organized and not so organized at the same time (my apologies); this is a side effect of Windows as a Service (WaaS) era - continuous development, continuous refinements, ongoing hunt for accessibility and usability issues.

Enough musings - if I understand the proposed code changes correctly:

1. While handling UIA notification events, NVDA may announce screen changes not caused by numer entry or results commands, and this is what is being handled (pasting from clipboard, for example).
2. NVDA will traverse through result element's children, looking for actual result text (using a for loop is smart).
3. NVDA will announce element content once actual result element is identified.

I don't expect new folks to be as detailed as above. I'll provide specific comments in the code fragments.

Thanks.

Hello Tristan, please use a descriptive title that explains what the PR does standalone.

@josephsl thanks so much for your insightful and kind comments. The code is quite good and I particularly appreciated how well commented it is. It must have taken a lot of work to get it to this point. This is my first time working with the NVDA project so I'm just not used to all the patterns involved.

Hello Tristan,

At least we got the basics of the fix right, and thank you for your willingness to think about our feedback/comments.

I'm known for writing lots of comments in my own code - maybe I can't "keep my mouth shut" while programming... I think programming is a variation on writing essays (more so if you think about Python syntax and grammar). After all, working on apps like Calculator gave me writing materials in graduate school (I am attending a university in western United States).

About the for loop logic: consider the following scenario:

1. User presses Escape to clear Calculator display string.
2. A UIA notification event is fired.
3. NVDA notices that it can announce the display string since the last entered command is not a number.
4. The just proposed code (for loop) runs, announcing "display is 0".
5. Somehow notification event is handled once again, either becuase Calculator fired it or NVDA itself is handling it (again).
6. The display string announcmenet may repeat.

The key is checking for value equality (== versus !=); a possible optimization is that NVDA doesn't have to fall back to default UIA notification event handling implementation (where it will report whatever the display string is as long as the element is focused on screen). This is what the call to "nextHandler" is - NVDA's event handling system could be viewed as an iterative "chain" where different modules can observe and handle events if they want to, otherwise default behavior (such as announcing things) will be performed. I think we can safely ignore this odd thing in this PR.

If you want this to be part of NVDA 2024.3, I advise adding a changelog entry (we use Markdown syntax); if you want it in the next release after that, wait until 2024.3 enters beta phase (or shortly before) once NV Access people updates NVDA version in the source code. Then mark this PR as ready for review. Of course, since the issue may as well lie with Calculator executable itself, sending a "love letter" a.k.a. feedback to Microsoft from time to time about UIA notification event issue would be nice too.

Thanks.

Please add an entry to the changelog. The general approach here looks good to me for the code changes.

Done.

About the for loop logic: consider the following scenario:

1. User presses Escape to clear Calculator display string.
2. A UIA notification event is fired.
3. NVDA notices that it can announce the display string since the last entered command is not a number.
4. The just proposed code (for loop) runs, announcing "display is 0".
5. Somehow notification event is handled once again, either becuase Calculator fired it or NVDA itself is handling it (again).
6. The display string announcement may repeat.

Using speech.cancelSpeech() eliminates the issue of duplicated speech announcements from UIA notifications. In fact, when pasting into the calculator, this is exactly what happens -- you get two notifications. So that's why I added the cancelSpeech() call.

 __________________________________________________________________________________________________________________________________________
< Test early. Test often. Test automatically. Tests that run with every build are much more effective than test plans that sit on a shelf. >
 ------------------------------------------------------------------------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?