Add an appModule for zoom, fixes #7754#8491
Merged
Merged
Conversation
Contributor
|
Hi, Hmmm, if what's new entry is added, this could set a precedent for others. @LeonarddeR and others, any comments on this? Thanks.b |
|
* Hmmm, if what's new entry is added, this could set a precedent for others
I’m not sure what this means. Do you not want people to know that such a module was added?
|
Contributor
|
Hi, in the past, when folks added what’s new entries, it caused merge conflicts, thus these were added right after the pull request in question was merged into master. Thanks.
|
|
Ah. Thank you for that explanation.
|
LeonarddeR
previously approved these changes
Jul 10, 2018
LeonarddeR
left a comment
Collaborator
There was a problem hiding this comment.
Note that I honestly haven't used this software, however I assume you've tested your implementation.
| - NVDA now asks if it should save configuration and restart if the user has just changed the language in the User Interface Settings Dialog. NVDA must be restarted for the language change to fully take effect. | ||
| - If a synthesizer can not be loaded, when choosing it from the synthesizer dialog, a message box alerts the user to the fact. | ||
| - When loading a synthesizer for the first time, NVDA lets the synthesizer choose the most suitable voice, rate and pitch parameters, rather than forcing it to defaults it thinks are ok. This fixes a problem where Eloquence and Viavoice sapi4 synths start speaking way too fast for the first time. | ||
|
|
Collaborator
There was a problem hiding this comment.
It looks like this line got accidentally removed
Contributor
|
Hi, one more thing: could you mind adding copyright headers please? Thanks.
|
feerrenrut
reviewed
Jul 10, 2018
feerrenrut
left a comment
Contributor
There was a problem hiding this comment.
Also, I will have to rely on your testing. Can you get the original reporter to confirm this addresses the issue for them with a try/pr build?
| - NVDA now asks if it should save configuration and restart if the user has just changed the language in the User Interface Settings Dialog. NVDA must be restarted for the language change to fully take effect. | ||
| - If a synthesizer can not be loaded, when choosing it from the synthesizer dialog, a message box alerts the user to the fact. | ||
| - When loading a synthesizer for the first time, NVDA lets the synthesizer choose the most suitable voice, rate and pitch parameters, rather than forcing it to defaults it thinks are ok. This fixes a problem where Eloquence and Viavoice sapi4 synths start speaking way too fast for the first time. | ||
|
|
Collaborator
Author
|
Copyright headers are there. Yes, will do. |
Collaborator
Author
|
I rebased this on master. |
Collaborator
Author
|
I didn't mean to remove that line, sorry, I was messing with the online github file editor. This poses the question, why is there two lines at the end of that file though? |
Collaborator
Author
|
@feerrenrut multiple reports on the parent ticket now confirm my try build works as expected. |
feerrenrut
previously approved these changes
Jul 18, 2018
feerrenrut
approved these changes
Jul 18, 2018
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
6 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.
Link to issue number:
Fixes #7754
Summary of the issue:
Make zoom's notifications via alerts accessible.
Description of how this pull request fixes the issue:
In zoom, alerts are filtered out because zoom uses a window which is not a child of the foreground window for zoom. This pull request requests events for the window class the notifications use, even in the background.
Testing performed:
Tested to see if alerts worked while in zoom.
Known issues with pull request:
None
Change log entry:
Section: Bug fixes