Create a MessageDialog API

[SaschaCowley]

Link to issue number:

Closes #13007

Summary of the issue:

Currently, there are several ways to create message boxes/dialogs in NVDA, all of them with their own drawbacks.
Additionally, subclassing wx.Dialog (or using it directly) brings with it the risk of breaking parts of NVDA. See #13007 for in-depth discussion.

Thus, a new API for creating highly customizable dialogs that work well with NVDA is desired.

Description of user facing changes

This PR does not, in itself, introduce any end-user facing changes. However, it lays the groundwork for closing a number of issues.

Description of development approach

Subclassed wx.Dialog to add the desired functionality in #13007

Features and tasks

• Rename buttons
• Add custom buttons
• Associate context help with the given message box
• Show the dialog as a modal or a non-modal
• for non modal dialogs:
  • provide a default return code when dialogs are forced to close (eg via an exit, perform cancel) (WIP)
  • attach a callback to handle return codes
  • refocus a dialog in the event of needing to close it
  • if a default return code is provided, make the dialog close-able with alt+F4 and escape (WIP)
• for modal dialogs:
  • if wx.CANCEL is provided, make the dialog close-able with alt+F4 and escape (mostly done)
  • nice to have:
    • if wx.CANCEL is provided, perform this action when dialogs are forced to close.
    • refocus a dialog in the event of needing to close it
• Ensure that the test Quits from keyboard with about dialog open is re-enabled and works.
• Block exiting NVDA with modal MessageDialogs open.
• Focus open MessageDialogs without default actions when exiting.
• Tests
  • Unit tests
  • System tests
  • Manual test plan and necessary scripts
• Document the new API in the developer guide
• Migrate the new API from gui.MessageDialog to gui.message.
• Mark other means of creating message dialogs as deprecated.
  • Re implement their internals to use the new API
    • gui.message.messageBox
    • gui.nvdaControls.MessageDialog
    • gui.nvdaControls._ContinueCancelDialog

Testing strategy:

Unit, system and manual tests (TBD)

Message box shim tested with a variety of inputs and unittests. This is a fairly straightforward adapter.

nvdaControls.MessageDialog tested that screen curtain and add-on install warning dialogs still work as expected. Its private subclass, _ContinueCancelDialog, was tested by attempting to run the CRFT.

Known issues with pull request:

Internal dialogs are yet to be migrated. This should be done gradually before the existing means of creating message dialogs are removed.

Slightly odd behaviour when a callback function blocks, and focusBlockingInstances is called, where a hidden dialog is focused. This can be worked around, but possibly not without unintended consequences.

Some invalid combinations (namely a button which is set as the default action but set not to close the dialog) are silently changed. While documenting this should be sufficient, logging or raising an exception may be warranted.

Visual proportions of the re-implemented About dialog are different.

The ctrl+c behaviour of copying the dialog's title, contents and buttons is not supported.

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

[josephsl]

Hello Sascha,

With gui.messageBox reimplemented on top of gui.messageDialog.MessageDialog, here is the traceback I got when trying to instal an add-on that calls gui.messageBox in its onInstall task function:

ERROR - unhandled exception (01:32:50.339) - systemUtils.ExecAndPump(<function installAddonBundle at 0x03AD2668>) (23372):
Traceback (most recent call last):
File "wx\core.pyc", line 3427, in
wx._core.wxAssertionError: C++ assertion "wxThread::IsMain()" failed at ....\src\msw\thread.cpp(1432) in wxMutexGuiLeaveOrEnter(): only main thread may call wxMutexGuiLeaveOrEnter()!
ERROR - unhandled exception (01:33:04.024) - systemUtils.ExecAndPump(<function installAddonBundle at 0x03AD2668>) (23372):
wx._core.wxAssertionError: C++ assertion "wxThread::IsMain()" failed at ....\src\msw\thread.cpp(1432) in wxMutexGuiLeaveOrEnter(): only main thread may call wxMutexGuiLeaveOrEnter()!

The above exception was the direct cause of the following exception:

SystemError: <class 'wx._core.ActivateEvent'> returned a result with an exception set

Relevant ad-on code fragment (not the exact code, but is written similar to this):

minimumWinVer = winVersion.WinVersion(10, 0, 27800, releaseName="Windows 11 Selenium")
if winVersion.getWinVer() < minimumWinVer:
gui.messageBox(errorMessage, error Title, wx.OK | wx.ICON_ERROR)
raise RuntimeError("Windows version requirement not met")

Thanks.

[SaschaCowley]

@josephsl I believe the exception you encountered in #17304 (comment) should be resolved now. Would you mind testing again with at least fca7eb8?

Regarding the errors encountered when calling MessageDialog.ShowModal from a non-main thread, this is expected behaviour. wx.CallAfter should be used instead. Do you, @seanbudd , or others, think that this should detect if being called in a non-main thread and correct for that?

[josephsl]

Hi,

The error is resolved.

As for main thread detection: perhaps we could use main thread detection. However, it might be possible that an operation is performed right after dismissing this dialog that might block the main thread i.e. the dialog asks to perform a task that could take a long time to do such as network connectivity.

Thanks.

[josephsl]

Hi,

As a follow-up: the error still happens if gui.messageDialog.MessageDialog.ShowModal() is called directly as proposed in the message dialog notes posted a few days ago, so it might be worth looking into main GUI thread detection. Another way could be reminding folks to wrap ShowModal call inside wx.CallAfter function (or rather, define the dialog and the ShowModal call inside a dedicated function and call that function as an argument of wx.CallAfter).

Thanks for considering our feedback in implementingg this PR.