Skip to content

Import keyLabels only after translation is initialized to make the labels displayed in the user's language#14658

Merged
seanbudd merged 2 commits into
nvaccess:betafrom
lukaszgo1:againLocalizedKeyLabels
Feb 22, 2023
Merged

Import keyLabels only after translation is initialized to make the labels displayed in the user's language#14658
seanbudd merged 2 commits into
nvaccess:betafrom
lukaszgo1:againLocalizedKeyLabels

Conversation

@lukaszgo1

@lukaszgo1 lukaszgo1 commented Feb 21, 2023

Copy link
Copy Markdown
Contributor

Opened against beta, since this pull request fixes regression which ideally should not get into the release.

Link to issue number:

Fixes #14657

Summary of the issue:

In pull request #14528 keyLabels were imported pretty early during NVDA's startup. Since this module contains localized strings at the module level these are translated before languageHandler is initialized. This means that they're translated either to the default language of the system, or for locales for which Python's locale.getdefaultlocale fails they remain in English, disregarding the language set in preferences.

Description of user facing changes

Key labels are once again presented in the language set in preferences.

Description of development approach

  1. keyLabels are imported lazily in the configFlags
  2. I've added docstring to the keyLabels module explaining when it can be safely imported.

Testing strategy:

On a English Windows set NVDA's language to Polish, ensured that key labels in the keyboard help and in the preferences are displayed in Polish.

Known issues with pull request:

While this pull request fixes this particular issue I'm of the opinion that we should not initialize a fake translation before parsing user's configured language. Since this would be bigger change, unsuitable for the beta branch at this stage, I'm going to open a separate issue to discuss and track this. Edit: I've created #14660

Change log entries:

None needed - unreleased regression

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@lukaszgo1 lukaszgo1 requested a review from a team as a code owner February 21, 2023 19:35
@lukaszgo1 lukaszgo1 requested review from seanbudd and removed request for a team February 21, 2023 19:35
@lukaszgo1

Copy link
Copy Markdown
Contributor Author

CC @CyrilleB79 You may want to review this PR.

@seanbudd seanbudd added this to the 2023.1 milestone Feb 21, 2023
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 0ce1c3d87a

@CyrilleB79 CyrilleB79 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lukaszgo1 for having noticed this issue and for your fix.

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit b47f4fce56

@seanbudd seanbudd merged commit 3fe762c into nvaccess:beta Feb 22, 2023
@lukaszgo1 lukaszgo1 deleted the againLocalizedKeyLabels branch February 22, 2023 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants