D25984: Load translations

[Kai Uwe Broulik]

broulik abandoned this revision.
broulik added a comment.


  Looks like D27595  fixes the issue

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid
Cc: vkrause, mart, davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, 
dkardarakos, ngraham, apol, ahiemstra

D25984: Load translations

[Volker Krause]

vkrause added a comment.


  In D25984#589426 , @mart wrote:
  
  > ping, what's the current status of this?
  
  
  There's also https://phabricator.kde.org/D27595, which might address the 
same/a similar issue.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid
Cc: vkrause, mart, davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, 
dkardarakos, ngraham, apol, ahiemstra

D25984: Load translations

[Marco Martin]

mart added a comment.


  ping, what's the current status of this?

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: mart, davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, 
dkardarakos, ngraham, apol, ahiemstra, hein

D25984: Load translations

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D25984#578306 , @kossebau wrote:
  
  > Hm, though now I am confused why calling this from 
`KirigamiPlugin::initializeEngine` fixes things for you, the static struct 
constructor of that startup hook technology in the lib should be invoked before 
during loading, no?
  
  
  Looking at the ECM generated code, that one sees to do the catalog loading in 
the main thread, and if not in the main thread already using a timer event to 
trigger the loading there.
  While the plugin is loaded in another thread (called "QQmlThread" by what my 
debug output told me).
  
INIT CATALOG HOOKUP QThread(0x564b8c8c6fb0, name = "QQmlThread")
INIT ENGINE QThread(0x564b8c3a28d0)
LOADING FROM CATALOG HOOKUP QThread(0x564b8c3a28d0)
  
  Which explains to me why your patch then fixes things.
  
  No idea/proposal about/for a proper solution, but at least my curiousity is 
satisfied for now for what is broken here :/

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ngraham, apol, ahiemstra, mart, hein

D25984: Load translations

[Kai Uwe Broulik]

broulik added a comment.


  I guess it builds the QML tree and evaluates the `qsTr` calls and then finds 
it needs an item from a plugin and loads the import and only then translates 
it. Subsequent controls are translated, only the ones on the initial item are 
not. Perhaps if we shoehorn a `QQmlEngine::retranslate()` call in the ecm qm 
loader thing (or rather have a ecm qm loader for qml plugin) it should force 
re-evaluation of those things. Not pretty either :/

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ngraham, apol, ahiemstra, mart, hein

D25984: Load translations

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Hm, though now I am confused why calling this from 
`KirigamiPlugin::initializeEngine` fixes things for you, the static struct 
constructor of that startup hook technology in the lib should be invoked before 
during loading, no?
  
  Time to go playing with the actual code myself.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ngraham, apol, ahiemstra, mart, hein

D25984: Load translations

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D25984#578299 , @broulik wrote:
  
  > Ok, so turns out, it does call it when the plugin is loaded, but the 
translations are only installed *after* the QML has loaded and processed.
  
  
  Uh, okay, that seems like things need indeed another approach. Actually, any 
librarries using the hook and linked to only from qml plugins would then be 
prone to fail?

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ngraham, apol, ahiemstra, mart, hein

D25984: Load translations

[David Edmundson]

davidedmundson added a comment.


  > Okay. Which app did you test with? Does it link kcoreaddons? Would poke 
then as well a bit, as I happen to have a related rant blog post lying around 
;) ).
  
  systemsettings5 exhibits this situation

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ngraham, apol, ahiemstra, mart, hein

D25984: Load translations

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D25984#578298 , @broulik wrote:
  
  > Well, whatever it is, it doesn't work. My search field isn't translated and 
I put breakpoints and I only get it load translations for kcoreaddons and 
kjobwidgets.
  
  
  Okay. Which app did you test with? Does it link kcoreaddons? Would poke then 
as well a bit, as I happen to have a related rant blog post lying around ;) ).

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

D25984: Load translations

[Kai Uwe Broulik]

broulik added a comment.


  Ok, so turns out, it does call it when the plugin is loaded, but the 
translations are only installed *after* the QML has loaded and processed.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

D25984: Load translations

[Kai Uwe Broulik]

broulik added a comment.


  Well, whatever it is, it doesn't work. My search field isn't translated and I 
put breakpoints and I only get it load translations for kcoreaddons and 
kjobwidgets.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

D25984: Load translations

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D25984#578296 , @broulik wrote:
  
  > We don't link against Kirigami, so the startup routine stuff doesn't apply?
  
  
  `Q_COREAPP_STARTUP_FUNCTION` generates a static struct in the compiled code, 
whose constructor will be invoked on loading the lib which bundles that obect 
code, and the constructor does the registration call (which then also cares for 
directly executing the method if already passed QApp construction phase). See 
below for how exactly.
  
  So at least to what I "know" being loaded in a lib only later at runtime 
still should result in the call being triggered, as also at that time any 
static objects will be constructed/init during lib load.
  
  > And we're way after the QCoreApp constructor at this point.
  
  Yes, but that is something `qAddPreRoutine` deals with, by in that case then 
also executing the method itself:
  
void qAddPreRoutine(QtStartUpFunction p)
{
QStartUpFuncList *list = preRList();
if (!list)
return;
if (QCoreApplication::instance())
p();
// Due to C++11 parallel dynamic initialization, this can be called
// from multiple threads.
QMutexLocker locker();
list->prepend(p); // in case QCoreApplication is re-created, see 
qt_call_pre_routines
}
  
  See how `qAddPreRoutine` code is generated  by `Q_COREAPP_STARTUP_FUNCTION`:
  
#define Q_COREAPP_STARTUP_FUNCTION(AFUNC) \
static void AFUNC ## _ctor_function() {  \
qAddPreRoutine(AFUNC);\
} \
Q_CONSTRUCTOR_FUNCTION(AFUNC ## _ctor_function)
  
  with `Q_CONSTRUCTOR_FUNCTION` being:
  
#ifndef Q_CONSTRUCTOR_FUNCTION
# define Q_CONSTRUCTOR_FUNCTION0(AFUNC) \
namespace { \
static const struct AFUNC ## _ctor_class_ { \
inline AFUNC ## _ctor_class_() { AFUNC(); } \
} AFUNC ## _ctor_instance_; \
}
# define Q_CONSTRUCTOR_FUNCTION(AFUNC) Q_CONSTRUCTOR_FUNCTION0(AFUNC)
#endif

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

D25984: Load translations

[Kai Uwe Broulik]

broulik added a comment.


  We don't link against Kirigami, so the startup routine stuff doesn't apply? 
And we're way after the QCoreApp constructor at this point.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

D25984: Load translations

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D25984#578291 , @broulik wrote:
  
  > Qt docs say "Adds a global function that will be called from the 
QCoreApplication constructor. " but when we at runtime somewhere load a QML 
plugin we're too late with that.
  
  
  Q_COREAPP_STARTUP_FUNCTION creates code using qAddPreRoutine(), which does 
directly execute the registered method though when the QApp already exists, as 
I tried to hint before, and has since Qt 5.6 at least. (see the above link :) 
). The API dox is missing to define that, indeed, but thinking about it one 
would expect it to happen.
  
  Things might fail though if there is static linking involved perhaps with the 
plugins somehow?

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

D25984: Load translations

[Kai Uwe Broulik]

broulik added a comment.


  Qt docs say "Adds a global function that will be called from the 
QCoreApplication constructor. " but when we at runtime somewhere load a QML 
plugin we're too late with that.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

D25984: Load translations

[Friedrich W. H. Kossebau]

kossebau added a comment.


  > The ECM QM Loader uses Q_COREAPP_STARTUP_FUNCTION which doesn't work for 
things loaded as plugins like QML modules.
  
  Curious, where it exactly does it fail to work? Q_COREAPP_STARTUP_FUNCTION 
being nvoked during lib loading after the QApp instance exists still should 
result in the startup function to be executed from what I remember and just 
looked at again in the code 
(https://code.woboq.org/qt5/qtbase/src/corelib/kernel/qcoreapplication.cpp.html#286),
 see the
  
if (QCoreApplication::instance())
p();
  
  So, what am I missing? :)

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

D25984: Load translations

[Kai Uwe Broulik]

broulik added a comment.


  Sounds good to me lol

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

D25984: Load translations

[Harald Sitter]

sitter added a comment.


  Seems a bit meh to have a verbatim copy of the auto generated functions. 
Perhaps it'd be better to add an option to the ECM generator to export a loader 
function for use in plugins?
  Then we could just call that from the plugin init instead of having to copy 
it.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

To: broulik, #kirigami, #frameworks, kossebau, aacid, sitter
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

D25984: Load translations

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Kirigami, Frameworks, kossebau, aacid.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  The ECM QM Loader uses `Q_COREAPP_STARTUP_FUNCTION` which doesn't work for 
things loaded as plugins like QML modules.
  This copies the logic over to the plugin so the translations are loaded when 
the QML plugin is loaded.
  Not really like it, so I'm open to suggestions how to do that better in ECM 
or I don't know?

TEST PLAN
  - The Search field in Kirigami is translated now
  - The AboutPage in Kirigami is translated now

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25984

AFFECTED FILES
  src/kirigamiplugin.cpp
  src/kirigamiplugin.h

To: broulik, #kirigami, #frameworks, kossebau, aacid
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein