API dox: add note about calling setApplicationDomain after QApp creation
ClosedPublic

Authored by kossebau on Oct 17 2017, 10:22 PM.

Details

Summary

The current implementation of KLocalizedString::setApplicationDomain()
will trigger the creation of the KLocalizedStringPrivateStatics instance and
thus also the calculation of the locale languanges from the env vars
as set at that time, and QLocale::system() on some platforms.
So the claim that "This call can be made at any place in the code" does not
hold, instead API consumers should be instructed about when to call
the method ideally.

Diff Detail

Repository
R249 KI18n
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Oct 17 2017, 10:22 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 17 2017, 10:22 PM

Another option might be to split off a data structure where the application domain string is stored separately, and thus delay the initialization of the KLocalizedStringPrivateStatics instance to the first i18n call. Though given that the initialisation sets global values which then are const for the rest of the runtime based on the environment at the time, that would be slightly random.
Having straight control about the time when the init is done feels better to me. Possibly that should be even more emphasized in the API dox?

@ilic ping? Can you confirm this is correct and by design?

Motivation is:
KLocalizedString::setApplicationDomain() triggers the creation of the KLocalizedStringPrivateStatics instance, which then also reads the currently set env vars to collect the set locales/languages.
Now KXMLGUI's language switching support for programs injects the override language via Q_COREAPP_STARTUP_FUNCTION(initializeLanguages) (see kswitchlanguagedialog_p.cpp lines 44ff) by twisting the LANGUAGE env var.
So for that to work, the KLocalizedStringPrivateStatics instance has to be created only _after_ the QCoreApplication constructor is run.

A few programs had the order reversed, due to calling KLocalizedString::setApplicationDomain() before QApp creation, resulting in the language switching override being ignored and thus broken.

ilic added a comment.Oct 29 2017, 6:20 PM

Well... it's a tough situation. It is not by design that i18n calls should in any way depend on creation of QApplication, and also any library may place an i18n call before the main program creates QApplication. The only solution I see is that environment is rechecked at every i18n call. This would be easy to do (just replacing every s->languages with a newly implemented s->getLanguages()), but I've no idea what would be the performance hit of that.

dfaure accepted this revision.Dec 2 2017, 5:12 PM
dfaure added a subscriber: dfaure.

Yes.

any library may place an i18n call before the main program creates QApplication

No, that's invalid.
Before QApp is created, many things are not initialized, including the locale. The translation support can have the same requirement, then.

This revision is now accepted and ready to land.Dec 2 2017, 5:12 PM
In D8351#161562, @ilic wrote:

Well... it's a tough situation. It is not by design that i18n calls should in any way depend on creation of QApplication, and also any library may place an i18n call before the main program creates QApplication. The only solution I see is that environment is rechecked at every i18n call. This would be easy to do (just replacing every s->languages with a newly implemented s->getLanguages()), but I've no idea what would be the performance hit of that.

Thanks for reply, @ilic, sorry for not having picked up immediately, as I agree it's tough and I set the topic aside a little to have some thoughts develop in the back of my mind.

One problem that i see with rechecking the environment at every i18n call is that this potentially could result in inconsistent UI. Because the average applications using KI18n is not written to support switching UI localization on the fly. Incl. KXMLGUI's language switching support, which only injects its settings at a roughly defined point in time, as part of the post-handlers of the Q*App instance creation.

So I would agree with @dfaure that for the current KI18n usage (which is somewhat coupled with KXMLGUI's language switching support) we should just stick formally to what technically is needed right now: only doing UI locale-based things after the Q*App instance is created and thus after everything related to localization is setup and prepared (and will stay to the end of the Q*App instance lifetime.

So would push this change upcoming WE, Feb 10/11, finally, unless someone objects.

((In time for Qt6/KF6 we should perhaps revisit this and hopefully have some people work on adding proper infrastructure and usage patterns to allow such switching UI localization on the fly. It's pretty sad that Web apps, which are younger, are better here))

ilic added a comment.Feb 6 2018, 2:14 PM

Yes, I guess in the end it's the simplest way to look at it.

One thing though: I'd add at least another sentence claryfing that unlike an i18n* call, a ki18n* can happen at any time, and only its toString method should be called after Q*App instance creation (because that's when translation happens and locale is queried).

In D8351#201926, @ilic wrote:

Yes, I guess in the end it's the simplest way to look at it.

One thing though: I'd add at least another sentence claryfing that unlike an i18n* call, a ki18n* can happen at any time, and only its toString method should be called after Q*App instance creation (because that's when translation happens and locale is queried).

Makes sense to me, will see to add that (with some post-commit review ;) ). If you have some phrasing proposal, would be happy to have something to copy (lazy me).

This revision was automatically updated to reflect the committed changes.