[SaschaCowley]

Link to issue number:

None

Summary of the issue:

NVDA currently uses winsound.PlaySound in a few places, which is part of the winmm API.

Description of user facing changes

None.

Description of development approach

Replaced these calls with calls to winsound.MessageBeep, as they are all calls to play system sounds.

Testing strategy:

Tested:

• Pressing an unbound key in browse mode
• Logging a message at level critical

I was unable to test typing into a protected cell, as my version of Excel brings up a dialog if you attempt to type into or edit a protected cell.

Known issues with pull request:

None.

Code Review Checklist:

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

@coderabbitai summary

[seanbudd]

Could we also add a lint rule banning it: https://docs.astral.sh/ruff/rules/banned-api/

[CyrilleB79]

I guess that it means that add-ons should not use it either?

This should be documented in the changes for developers. And if possible, also explain why; even if NVDA does not use winmm at all anymore, it's not clear to me why it should not be used at all (e.g. installer logo removed).

In th

[cary-rowen]

There also appears to be a large number of speech synthesizer add-ons that rely on this API, meaning these synthesizers will be broken. If possible, could there be a more friendly documentation for add-on developers to explain or guide them on how to migrate to the new API?

[seanbudd]

@CyrilleB79 - supportive of your suggestions here, we just can't publish details of the security issue yet

[SaschaCowley]

@CyrilleB79, @seanbudd I don't think that this change needs to be documented in our changes for developers. We are just removing our own use of a function that has long been deprecated.

@cary-rowen I am a little confused. Do you want us to provide documentation on the Windows Core Audio APIs?

[cary-rowen]

I'm not sure, but I think we should reduce maintenance costs for add-on developers as much as possible. If it's me and my add-on isn't working, I know NV Access made API breaking changes, but I'd like to know what's recommended next? What should I do?

[SaschaCowley]

@cary-rowen we have made API breaking changes, but none of them stop add-on authors from using winmm. Add-on authors can still use whatever audio APIs they want to, we are just switching to Windows' core audio APIs internally. There is plenty of documentation on migrating from winmm to Windows core audio.

[LeonarddeR]

Regression found. See #18193