Add check for open messageBoxes to prevent exit#12984
Conversation
702f374 to
4c903c7
Compare
|
As inMessageBox is now used quite a bit more, including disallowing exit of NVDA, it might be an idea to put some further protections around this variable.
|
|
rebasing from master to beta |
0c36b04 to
d9d7e49
Compare
feerrenrut
left a comment
There was a problem hiding this comment.
I think this approach is ok for a temporary fix in 2021.3, but I'd like a more robust design for 2022.1. If we agree on this please update the description to make this clear.
The design for 2022.1 should allow the exit strategy to be set when the message box is created (eg: autoclose, bring to foreground). There seem to be different requirements for different messages.
I'm not sure if the following is necessary. Perhaps leave this out, and it can be considered in the follow up PR for 2022.1:
- ``gui.messageBox`` should be replaced with ``gui.message.messageBox`` in 2022.1. (#12984)
- ``gui.isInMessageBox`` should be replaced with ``gui.message.isInMessageBox()`` in 2022.1. (#12984)
This follows our pattern for the other releases this year, of warning about deprecating changes "early". |
Given the development of 2022.1 has already started, it's not really a warning. But more importantly I don't think we can be confident this is the approach we will follow, and I don't want to spend time blocking the beta to gain that confidence. |
|
To create two message boxes, in NVDA Python console:
```
wx.CallLater(300,wx.MessageBox,"Hello", "Goodbye"); wx.CallLater(1500,
wx.MessageBox, "Hello 2", "Goodbye 2")
```
I see the first one appear at 500 ms, then the second one appear over the top at 1500 ms. If I dismiss the second, the first is still behind it.
|
feerrenrut
left a comment
There was a problem hiding this comment.
Also, please update the changes entries in the description. I don't think we should recommend something for 2022.1 until we know for sure what it will be. If we can't make a recommendation then I don't think there needs to be something in the changes file.
Co-authored-by: Reef Turner <feerrenrut@users.noreply.github.com>
See test results for failed build of commit cdeeddfba0 |
Fix up of #12984 Summary of the issue: Backwards compatibility for the following code was broken in 2021.3 beta. from gui import isInMessageBox Note: this code creates a copy of the value of isInMessageBox, and as such, was only accurate if it was being imported when it was being checked. Importing like this also prevents gui.isInMessageBox from being updated: from gui import isInMessageBox isInMessageBox = False # this doesn't update gui.isInMessageBox Description of how this pull request fixes the issue: Creates an isInMessageBox that is updated by isInMessageBox. Note that while from gui import isInMessageBox can run without error now, using or updating the variable will not work as expected, nor has it ever worked properly.
…essageBoxActive (#13376) Relates to #13007, as the new design should be backwards compatible, we can implement the API changes as-is for 2022.1. Follow up to the API proposed in #12984 Summary of the issue: The API for gui.message had not yet been determined for 2022.1, as per #13007. As the future API is intended to be backwards compatible, this PR commits to the API proposed in #12984. Description of how this pull request fixes the issue: gui.IsInMessageBox was a boolean, this has been replaced by a function gui.message.isModalMessageBoxActive, which tracks if a messageBox is open in a safer manner. Changes logic gui/message.py to be safer with locking and handling errors.
Link to issue number:
Relates to #12976
Summary of the issue:
NVDA crashes if you attempt to exit NVDA with a messageBox open (eg the About Dialog).
This is due to wx Idle Events firing on the dialogs while they are being forced to close in the exit process.
Description of how this pull request fixes the issue:
Prevent exit while a MessageBox dialog is open - this is because the state of a MessageBox dialog is important and should be determined before exit.
Testing strategy:
NVDA menu > AboutKnown issues with pull request:
The state of the About Dialog is not important, and a different dialog that can be expected to close on exit should be used instead.
The messageBox module could be redesigned, but this is the safest solution while preserving backwards compatibility.
Change log entries:
Changes
Code Review Checklist: