New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a bug causing NVDA to use the wrong language when announcing selected/unselected symbols. #7687

Merged
merged 2 commits into from Dec 21, 2017

Conversation

Projects
None yet
4 participants
@devil418
Contributor

devil418 commented Oct 23, 2017

Summary of the issue:

When languages of the currently selected voice and of NVDA itself
differ, NVDA uses the voice language when announcing typed and erased
characters. This wasn't the case when selecting by character, the NVDA
language was always used.

Description of how this pull request fixes the issue:

There was a function called "getCurrentLanguage" already present in speech.py. It just needed to be used instead of LanguageHandler to determine the locale used when reading the selected text.

Testing performed:

I have tested this on the english language version of NVDA and polish espeak and I can confirm that it works correctly now.

Change log entry:

Bug Fixes
NVDA now uses the correct language when announcing symbols when text is selected.

Fix a bug causing NVDa to use the wrong language when announcing
selected/unselected symbols.

When languages of the currently selected voice and of NVDA itself
differ, NVDA uses the voice language when announcing typed and erased
characters. This wasn't the case when selecting by character, the NVDA
language was always used.

@ehollig ehollig requested a review from feerrenrut Oct 23, 2017

@ehollig ehollig added the quick fix label Oct 23, 2017

@feerrenrut

This comment has been minimized.

Contributor

feerrenrut commented Oct 25, 2017

Since there is no bug for this, could you please update this with steps to reproduce. Thanks.

@devil418

This comment has been minimized.

Contributor

devil418 commented Oct 25, 2017

Sorry if this is a duplicate, but it seems my email reply didn't go through, at least I can't see it on the page.
steps to reproduce:

  1. set your NVDA language to english.
  2. switch to espeak.
  3. set your voice to something different than english.
  4. type a symbol, i.e. ' tick or ; semicolon and note what espeak says (it will read the symbol in the right language). If you for example switch it to polish, it will read the polish pronunciation for that symbol.
  5. Press shift + left arrow to select the symbol, espeak will announce the english name of the symbol and the word selected. It should announce the name in the selected language, as it does when typing, erasing or reading, but it won't. I don't think it's exclusively an espeak problem, using the wrong language (by NVDA) seems to be the cause and this PR fixes that.
@feerrenrut

This comment has been minimized.

Contributor

feerrenrut commented Oct 31, 2017

Thanks for adding that.

@feerrenrut

Overall this looks ok. However this causes the unit tests to fail, so this will need to be updated before we can accept the PR. To run the tests run scons tests.

Also, I would like this to be tested with the new version of espeak that is currently incubating (on the next branch)

@feerrenrut feerrenrut removed the quick fix label Oct 31, 2017

@devil418

This comment has been minimized.

Contributor

devil418 commented Nov 1, 2017

I've just tested it with the next branch and, as expected, it works exactly the same because it's an NVDA issue that can be observed when using espeak but it's not unique to one synthesizer, it can be observed with other synths too, for example ivona2 on sapi5. That synth also communicates to NVDA about what language it speaks so the issue affects it too. I've provided my instructions for espeak because every NVDA instance has one available but this was only meant as an easily-reproducible example. I'l look into the unit tests too. At a first glance, it seems that now a synthesizer is required to determine the language and the tests fail because it's not available in the testing environment. Providing a mock and rewriting the relevant method so that it can handle this case properly and revert to the old behavior when needed seem like good approaches to solve this problem.

Fix the unit tests.
The previous commit broke the unit tests because getCurrentLanguage
needed a synthesizer to be set but it wasn't in the test environment.
Fixed the getCurrentLanguage method to not require a synthesizer and
fall back to using NVDA's language. when a synth is not available, just
as if it didn't declare what language it was using.

feerrenrut added a commit that referenced this pull request Dec 4, 2017

Incubate #7687
Merge remote-tracking branch 'origin/pr/7687' into next

@feerrenrut feerrenrut merged commit 2ce9bb3 into nvaccess:master Dec 21, 2017

1 check passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details

@nvaccessAuto nvaccessAuto added this to the 2018.1 milestone Dec 21, 2017

feerrenrut added a commit that referenced this pull request Dec 21, 2017

Update changes file for PR #7687
NVDA now uses the correct language when announcing symbols when text is selected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment