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

UI language: when changing NVDA's UI language, clicking cancel button at the warning dialog causes NVDA to present certain messages in the new language #4561

Closed
nvaccessAuto opened this Issue Oct 21, 2014 · 12 comments

Comments

Projects
None yet
5 participants
@nvaccessAuto

nvaccessAuto commented Oct 21, 2014

Reported by nvdakor on 2014-10-21 06:04
Hi,
If you "cancel" UI language switch, NVDA presents certain messages in the newly chosen language.

STR:

  1. Open General Settings, and change the UI language (say, from English to Spanish).
  2. When the warning dialog appears, click "cancel" instead of "OK".
    Expected: NVDA displays all prompts and setting labels using the previously selected language.
    Actual: Certain messages, such as setting labels and symbols are presented (spoken, shown, etc.) using the new language.

Technical: in GUI.SettingsDialogs, when leaving General Settings dialog, we don't check for wx.CANCEL, thus erroneously saving the new language setting, causing certain GUI elements to be shown in the new language. One possible workaround would be:

  1. Have a new variable in either config.conf or globalVars to indicate the language that NVDA should switch to upon restart. By default, it should be None, but when the user sets a new language, this variable will store the new language (the pending language change, that is).
  2. When destroying General Settings dialog instance, check if the user has pressed ENTER on cancel button in the new language warning dialog, and if so, store the new language in the pending language switch variable and don't mess with the langue setting in the config file until the user actually saves the newly changed config or restarts.
  3. In core.restart and during config save routines, check if there is a value in pending language switch variable, and if so, then set the language to the new lang value. If the value is None, assume that no lang change has occured (same routine as what we have now).

Thanks.

@nvaccessAuto

This comment has been minimized.

Show comment
Hide comment
@nvaccessAuto

nvaccessAuto Oct 21, 2014

Comment 1 by jteh on 2014-10-21 07:40
I think there is a simpler solution. Move the language check and dialog to the top of onOk. If the user cancels the dialog, return early without calling super so the dialog isn't dismissed. The only question is whether to temporarily call languageHandler.setLanguage for the new language while displaying the restart dialog so that the dialog can be displayed in the new language like we do now. I tend to think we actually shouldn't do this, since the user had to know the previous language to change the language anyway and much of the rest of the interface will still be displayed in the previous language before the restart.

nvaccessAuto commented Oct 21, 2014

Comment 1 by jteh on 2014-10-21 07:40
I think there is a simpler solution. Move the language check and dialog to the top of onOk. If the user cancels the dialog, return early without calling super so the dialog isn't dismissed. The only question is whether to temporarily call languageHandler.setLanguage for the new language while displaying the restart dialog so that the dialog can be displayed in the new language like we do now. I tend to think we actually shouldn't do this, since the user had to know the previous language to change the language anyway and much of the rest of the interface will still be displayed in the previous language before the restart.

@nvaccessAuto

This comment has been minimized.

Show comment
Hide comment
@nvaccessAuto

nvaccessAuto May 18, 2015

Comment 3 by nvdakor on 2015-05-18 23:20
Hi,
I'll try both appraoches in summer to see which one seems better. Thanks.

nvaccessAuto commented May 18, 2015

Comment 3 by nvdakor on 2015-05-18 23:20
Hi,
I'll try both appraoches in summer to see which one seems better. Thanks.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager Nov 18, 2015

Collaborator

The easiest fix is probably to not call setLanguage in onOk for the general settings dialog. This would mean that the language file is no longer validated before the restart, so the user could end up with a broken language when NVDA comes back up, except I think it automatically falls back to English in that case. IMO this is better than ending up with a partially translated interface even though the prompt said the change wouldn't take effect until you restart NVDA. Scons should maybe check that all stock language files, braille tables, etc are valid during the build process.

Collaborator

dkager commented Nov 18, 2015

The easiest fix is probably to not call setLanguage in onOk for the general settings dialog. This would mean that the language file is no longer validated before the restart, so the user could end up with a broken language when NVDA comes back up, except I think it automatically falls back to English in that case. IMO this is better than ending up with a partially translated interface even though the prompt said the change wouldn't take effect until you restart NVDA. Scons should maybe check that all stock language files, braille tables, etc are valid during the build process.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Nov 18, 2015

Contributor

Actually, looking at the text of the language change prompt, I agree the only thing we need to do is stop calling setLanguage.

Contributor

jcsteh commented Nov 18, 2015

Actually, looking at the text of the language change prompt, I agree the only thing we need to do is stop calling setLanguage.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Nov 18, 2015

Collaborator

Hi,

At least when the user presses Cancel. I’ll take a look at this tonight (perhaps a pull request to follow later).

Thanks for feedback.

From: James Teh [mailto:notifications@github.com]
Sent: Wednesday, November 18, 2015 2:24 PM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] UI language: when changing NVDA's UI language, clicking cancel button at the warning dialog causes NVDA to present certain messages in the new language (#4561)

Actually, looking at the text of the language change prompt, I agree the only thing we need to do is stop calling setLanguage.


Reply to this email directly or view it on GitHub #4561 (comment) .

Collaborator

josephsl commented Nov 18, 2015

Hi,

At least when the user presses Cancel. I’ll take a look at this tonight (perhaps a pull request to follow later).

Thanks for feedback.

From: James Teh [mailto:notifications@github.com]
Sent: Wednesday, November 18, 2015 2:24 PM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] UI language: when changing NVDA's UI language, clicking cancel button at the warning dialog causes NVDA to present certain messages in the new language (#4561)

Actually, looking at the text of the language change prompt, I agree the only thing we need to do is stop calling setLanguage.


Reply to this email directly or view it on GitHub #4561 (comment) .

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Nov 18, 2015

Contributor
Contributor

jcsteh commented Nov 18, 2015

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Nov 18, 2015

Collaborator

Hi,

Done. The below pull request should be enough.

Technical: Jamie is right: the simplest solution was to commenting out the whole set language try statement. I kept it in there for now (commented out) in case this isn’t the solution we are looking for, and please do remove the entire block once language communities validates this solution. Thanks.

Collaborator

josephsl commented Nov 18, 2015

Hi,

Done. The below pull request should be enough.

Technical: Jamie is right: the simplest solution was to commenting out the whole set language try statement. I kept it in there for now (commented out) in case this isn’t the solution we are looking for, and please do remove the entire block once language communities validates this solution. Thanks.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Nov 18, 2015

Collaborator

Hi,
I guess the above commit link could serve as an indirect pull request... (haven't found a way to attach pull requests to existing issues, and it appears the only way to do it is via CLI...).
Thanks.

Collaborator

josephsl commented Nov 18, 2015

Hi,
I guess the above commit link could serve as an indirect pull request... (haven't found a way to attach pull requests to existing issues, and it appears the only way to do it is via CLI...).
Thanks.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Nov 18, 2015

Contributor

Since you're a collaborator, you can just push a topic branch to the main repository. However, the general workflow is that you would create a pull request and reference this issue in the comment, as noted in the Contributing guide. The GitHub API can convert an issue to a pull request, but we don't want to depend on stuff you can't do from the UI and this also has other problems.

There's no need to comment code out; just remove it. We can always revert the commit if it breaks something (and I highly doubt it). That's the lovely thing about version control. :) The idea is that we should be able to merge a topic branch without changes if it works, not have to do a subsequent commit to remove stuff.

Contributor

jcsteh commented Nov 18, 2015

Since you're a collaborator, you can just push a topic branch to the main repository. However, the general workflow is that you would create a pull request and reference this issue in the comment, as noted in the Contributing guide. The GitHub API can convert an issue to a pull request, but we don't want to depend on stuff you can't do from the UI and this also has other problems.

There's no need to comment code out; just remove it. We can always revert the commit if it breaks something (and I highly doubt it). That's the lovely thing about version control. :) The idea is that we should be able to merge a topic branch without changes if it works, not have to do a subsequent commit to remove stuff.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Nov 18, 2015

Collaborator

Hi,

Done (as suggested). If this is acceptable, I propose squashing the branch when it is incubating or moves to master. Thanks.

Collaborator

josephsl commented Nov 18, 2015

Hi,

Done (as suggested). If this is acceptable, I propose squashing the branch when it is incubating or moves to master. Thanks.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager Nov 19, 2015

Collaborator

I also looked at this briefly last night, before reading this conversation. I didn't make any changes to the GUI but did clean up the comments in languageHandler.py a bit. So in case you'd like to incorporate that, it's in dkager/nvda@b2dd732.

Collaborator

dkager commented Nov 19, 2015

I also looked at this briefly last night, before reading this conversation. I didn't make any changes to the GUI but did clean up the comments in languageHandler.py a bit. So in case you'd like to incorporate that, it's in dkager/nvda@b2dd732.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Aug 16, 2016

Collaborator

Coming back to this...

@dkager: I think it might be best if we can combine our work.

Thanks.

Collaborator

josephsl commented Aug 16, 2016

Coming back to this...

@dkager: I think it might be best if we can combine our work.

Thanks.

@feerrenrut feerrenrut self-assigned this Sep 29, 2016

feerrenrut added a commit that referenced this issue Sep 30, 2016

No longer set the language before restarting
See issue #4561
When a new language is selected (on the general settings dialog) and the ok
button is pushed, the language is not changed until until NVDA is restarted.

This fixes an issue where some parts of the UI are translated and some
are not.

feerrenrut added a commit that referenced this issue Oct 4, 2016

incubates #6415
See issue #4561

Merge branch 'i4561_fixUiLangChange' into next

@nvaccessAuto nvaccessAuto added this to the 2016.4 milestone Oct 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment