From 095cacd668f9bc30f3b239e4a016bac6af697d42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20K=C3=B6hne?= Date: Tue, 29 Apr 2025 16:09:45 +0200 Subject: [PATCH] Do not treat the installation of an empty QTranslator as an error The return value of installTranslator() should indicate whether the _registration_ was successful. Apart from merely logging the result, the only sensible action the calling code might take is to delete the QTranslator object that failed to register. The old behavior of installTranslator() for empty QTranslator objects was to store the pointer for further use, but still return false. This might lead to hard-to-detect memory corruption. The patch changes behavior to _not_ consider installing empty QTranslator objects as an error. This has the following reasons: * As translations are often done later in the SW development process, it is common to replace empty .qm files with translated ones in a very late stage. But having different code paths based on this can be surprising. * The other parts of the QTranslator toolchain do not treat empty translations in a specific way, either. While at it, also remove the now unused QT_NO_TRANSLATION_BUILDER macro (left over from Qt 4). [ChangeLog][Core][Behavior Change] QCoreApplication::installTranslator() will now return true even for empty translators (QTranslator::isEmpty()). This makes it explicit that empty QTranslator objects are registered and queried, and therefore deleting the object prematurely might result in undefined behavior. Change-Id: Ia0531afcf5c72aa837406cf5de2cf73052014445 Reviewed-by: Masoud Jami Reviewed-by: Edward Welbourne --- src/corelib/kernel/qcoreapplication.cpp | 10 +++++--- .../kernel/qtranslator/tst_qtranslator.cpp | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index 6cf355aeaa9..180b8955906 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -2165,9 +2165,12 @@ void QCoreApplicationPrivate::quit() generated by \QD provide a \c retranslateUi() function that can be called. - The function returns \c true on success and false on failure. + The function returns \c true on success and \c false on failure. \note QCoreApplication does \e not take ownership of \a translationFile. + It is the responsibility of the application to ensure that, if the + function returned \c true, the \a translationFile object is alive until + either \l removeTranslator() is called for it, or the application exits. \sa removeTranslator(), translate(), QTranslator::load(), {Writing Source Code for Translation#Prepare for Dynamic Language Changes}{Prepare for Dynamic Language Changes} @@ -2180,16 +2183,15 @@ bool QCoreApplication::installTranslator(QTranslator *translationFile) if (!QCoreApplicationPrivate::checkInstance("installTranslator")) return false; + QCoreApplicationPrivate *d = self->d_func(); { QWriteLocker locker(&d->translateMutex); d->translators.prepend(translationFile); } -#ifndef QT_NO_TRANSLATION_BUILDER if (translationFile->isEmpty()) - return false; -#endif + return true; #ifndef QT_NO_QOBJECT QEvent ev(QEvent::LanguageChange); diff --git a/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp b/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp index eedf59a7c99..66dcac85f6b 100644 --- a/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp +++ b/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp @@ -75,6 +75,7 @@ private slots: void loadLocale_data(); void loadLocale(); void threadLoad(); + void install(); void testLanguageChange(); void plural(); void translate_qm_file_generated_with_msgfmt(); @@ -285,6 +286,30 @@ void tst_QTranslator::threadLoad() QVERIFY(thread.wait(10 * 1000)); } +void tst_QTranslator::install() +{ + { + // normal translation + QTranslator tor; + QVERIFY(tor.load("hellotr_la.qm")); + QCOMPARE(qApp->installTranslator(&tor), true); + QCOMPARE(qApp->removeTranslator(&tor), true); + } + { + // empty translation + QTranslator tor; + QVERIFY(tor.load("hellotr_empty.qm")); + QCOMPARE(qApp->installTranslator(&tor), true); + QCOMPARE(qApp->removeTranslator(&tor), true); + } + { + // nullptr + QCOMPARE(qApp->installTranslator(nullptr), false); + QCOMPARE(qApp->removeTranslator(nullptr), false); + } + +} + void tst_QTranslator::testLanguageChange() { languageChangeEventCounter = 0;