Revert message dialog API#17561
Merged
Merged
Conversation
This reverts commit 2477780.
michaelDCurran
approved these changes
Dec 20, 2024
7 tasks
SaschaCowley
added a commit
that referenced
this pull request
Jan 8, 2025
Closes #13007 Closes #12344 Closes #12353 Summary of the issue: This is the second PR to merge the message dialog API into NVDA. The previous merge was at `f437723`. See #17304 for full implementation details. The original PR was reverted by #17561, due to #17553 and #17560. 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. Tasks - [x] Fix the COM registration fixing tool (#17560) - [x] Fix NVDA updates (#17553) Description of development approach Restored `gui.nvdaControls.MessageDialog` inheriting from `ContextHelpMixin`, which was accidentally lost. Changed `updateCheck.UpdateAskInstallDialog.onUpdateButton` and `onPostponeButton` to simply end the modal, returning the appropriate code. Added a static method to `UpdateAskInstallDialog` which returns a callback function which, when given the return code from `UpdateAskInstallDialog`, performs the appropriate action. Added a property method to generate this callback function given the attributes of the instance on which it is called. Testing strategy: Created a portable copy of NVDA (`scons dist`), and tested running the CRFT: - Running and granting permission - works as expected - Running and denying permission - works as expected - Cancelling - works as expected Added `updateVersionType="snapshot:alpha" to `source/versionInfo.py`, and tested updating with NVDA running from source: 1. Attempt updating via NVDA menu -> Help -> Check for update... -> Download update -> Update - works as expected 2. Update by downloading then postponing update, then exit NVDA -> Install pending update a. Without incompatible updates installed - works as expected b. With incompatible updates installed - works as expected 3. Update by downloading then postponing update, restarting NVDA, then NVDA menu -> Install pending update - works as expected 4. Update by downloading then postponing update, restarting NVDA, then accepting the prompt to install the update - works as expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts PR
Reverts #17304
Issues fixed
Fixes #17560
Fixes #17553
Issues reopened
Reopens #13007
Reopens #12344
Reopens #12353
Reason for revert
This PR broke two important NVDA functions, and there is insufficient time before the Christmas/New Years break to fix the issues, so we are temporarily reverting so that alphas work over the break:
gui.message.MessageDialog's inheritance fromContextHelpMixinwas accidentally removed.runScriptModalDialogwas modified to increment and decrement_messageBoxCounter, butUpdateAskInstallDialogattempts to restart NVDA before it has been closed, so the update fails as NVDA appears to be in an unsafe state.Can this PR be reimplemented? If so, what is required for the next attempt
Yes:
MessageDialogso it inherits fromContextHelpMixincorrectly.runScriptModalDialog; or