Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix static localized text #3400

Merged
merged 1 commit into from
Nov 4, 2017
Merged

Fix static localized text #3400

merged 1 commit into from
Nov 4, 2017

Conversation

tobiasdiez
Copy link
Member

I had some problems with not initialized resource bundles (an issue on my side), which lead me to discover a few instances where localized text were accessed statically. Such text would never be translated correctly. Should be fixed with this PR. (I didn't add a changelog entry since it is a really minor fix.)

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Nov 4, 2017
@Siedlerchr Siedlerchr merged commit c7431f7 into master Nov 4, 2017
@Siedlerchr Siedlerchr deleted the fixStaticLocal branch November 4, 2017 11:08
Siedlerchr added a commit that referenced this pull request Nov 4, 2017
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into linuxlink
2f31be2 
* upstream/master: (26 commits)
  Fix static localized text (#3400)
  Fix problems in source editor (#3398)
  Fix MathSciNet fetcher (#3402)
  Fix NPE when saving new file
  Improve SyncLang.py (#3184)
  Config intellj code style to adhere to empty lines checkstyle rule (#3365)
  Don't list same look and feels more than once (#3393)
  progessdialog and result table are now shown correclty on copy files
  Export pdf/linked files  (#3147)
  Added checking integrity dialog (#3388)
  Update richtext and flowless (#3386)
  Source tab: hide notification pane when code is correct again (#3387)
  Titles are optional at crossref (#3378)
  Update entry for every change in the source panel (#3366)
  Rework AutoSetFileLinks (#3368)
  revert wrongly commited changes
  Refactorings (#3370)
  Fix freezing when running cleanup file operations like rename (#3315)
  This additional style setting for IDEA (#3355)
  Fix checkstyle
  ...

# Conflicts:
#	CHANGELOG.md
@halirutan
Copy link
Collaborator

The PR looks OK to me, although it's only cosmetics and not necessary. I just wanted to point out that these static texts should correctly be translated after my localization PR that cached the messages. The idea behind statically caching all messages was that it's anyway not possible to make a complete language change in the GUI without restarting JabRef. We always had the warning for the user to restart JabRef after setting a new language.

Back then, I saw these static initializations as well and the main issue with them was, that they were initialized before the localization framework was initialized with the correct language. Therefore, in former times there was a hidden, but non-critical bug. In main:

  1. The preferences were initialized. In them, at least one of the files you fixed was statically instantiated.
  2. Since the localization was not ready at this point, it used English as fallback
  3. Therefore, these things were statically initialized with English everytime no matter of the user language

I, therefore, reordered the init phase so that the language and localization were ready when the first access to it happened. This ensures correct language for the static things I found.

Nevertheless, this is a good PR and although I haven't tested it, it looks correct. If you like, you can refactor the initialization phase again which means moving

Localization.setLanguage(prefs.get(LANGUAGE, "en"));

from org.jabref.preferences.JabRefPreferences#JabRefPreferences back to line 71 in JabRefMain.

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

@LinusDietz LinusDietz LinusDietz approved these changes

Assignees
No one assigned
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tobiasdiez @halirutan @LinusDietz @Siedlerchr