Qmldebugtranslator: report elide issues correctly inside layouts

Task-number: QTBUG-96991
Pick-to: 6.2
Change-Id: I911044893fb6eac54c6fb8f2b236f422bd04a7ae
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/plugins/qmltooling/qmldbg_preview/qqmldebugtranslationservice.cpp

@@ -241,10 +241,10 @@ public:
         emit q->messageToClient(q->name(), packet.data());
     }
 
-    void sendMissingTranslations()
+    void sendTranslationIssues()
     {
         QVersionedPacket<QQmlDebugConnector> packet;
-        packet << Reply::MissingTranslations;
+        packet << Reply::TranslationIssues;
 
         QVector<TranslationIssue> issues;
         for (auto &&information : qAsConst(objectTranslationBindingMultiMap)) {
@@ -255,6 +255,18 @@ public:
                 issue.language = proxyTranslator->currentUILanguages();
                 issues.append(issue);
             }
+
+            QObject *scopeObject = information.scopeObject;
+            QQuickText *quickText = static_cast<QQuickText*>(scopeObject);
+            if (quickText) {
+                if (quickText->truncated()) {
+                    TranslationIssue issue;
+                    issue.type = TranslationIssue::Type::Elided;
+                    issue.codeMarker = codeMarker(information);
+                    issue.language = proxyTranslator->currentUILanguages();
+                    issues.append(issue);
+                }
+            }
         }
         std::sort(issues.begin(), issues.end(), [](const auto &l1, const auto &l2){
             return l1.codeMarker < l2.codeMarker;
@@ -263,20 +275,6 @@ public:
         emit q->messageToClient(q->name(), packet.data());
     }
 
-    void sendElidedTextWarning(const TranslationBindingInformation &information)
-    {
-        QVersionedPacket<QQmlDebugConnector> packet;
-        packet << Reply::TextElided;
-
-        TranslationIssue issue;
-        issue.type = TranslationIssue::Type::Elided;
-        issue.codeMarker = codeMarker(information);
-        issue.language = proxyTranslator->currentUILanguages();
-
-        packet << issue;
-        emit q->messageToClient(q->name(), packet.data());
-    }
-
     QQmlDebugTranslationServiceImpl *q;
 
     bool watchTextElides = false;
@@ -335,8 +333,8 @@ QQmlDebugTranslationServiceImpl::QQmlDebugTranslationServiceImpl(QObject *parent
             d, &QQmlDebugTranslationServicePrivate::sendLanguageChanged,
             Qt::QueuedConnection);
 
-    connect(this, &QQmlDebugTranslationServiceImpl::missingTranslations,
+    connect(this, &QQmlDebugTranslationServiceImpl::translationIssues,
-            d, &QQmlDebugTranslationServicePrivate::sendMissingTranslations,
+            d, &QQmlDebugTranslationServicePrivate::sendTranslationIssues,
             Qt::QueuedConnection);
 
     connect(this, &QQmlDebugTranslationServiceImpl::sendTranslatableTextOccurrences,
@@ -374,8 +372,8 @@ void QQmlDebugTranslationServiceImpl::messageReceived(const QByteArray &message)
             emit stateList();
             break;
         }
-        case QQmlDebugTranslation::Request::MissingTranslations: {
+        case QQmlDebugTranslation::Request::TranslationIssues: {
-            emit missingTranslations();
+            emit translationIssues();
             break;
         }
         case QQmlDebugTranslation::Request::TranslatableTextOccurrences: {
@@ -415,57 +413,6 @@ void QQmlDebugTranslationServiceImpl::engineAboutToBeRemoved(QJSEngine *engine)
     emit detachedFromEngine(engine);
 }
 
-QString QQmlDebugTranslationServiceImpl::foundElidedText(QObject *textObject, const QString &layoutText, const QString &elideText)
-{
-    Q_UNUSED(layoutText)
-    QString elidedTextResult = elideText;
-    // do the check only for text objects which have translation bindings
-    auto it = d->objectTranslationBindingMultiMap.find(textObject);
-    if (it != d->objectTranslationBindingMultiMap.end()) {
-        if (QQuickItem* quickItem = qobject_cast<QQuickItem*>(textObject)) {
-            const TranslationBindingInformation information = d->objectTranslationBindingMultiMap.value(quickItem);
-
-            QQuickItem* parentItem = quickItem->parentItem();
-            QString parentTypeName = parentItem->metaObject()->className();
-
-            // Currently text fields inside a layout give false signals about elides
-            // so we just omit them
-
-            if (d->watchTextElides && !parentTypeName.endsWith("Layout")) {
-                d->sendElidedTextWarning(information);
-            }
-
-            if (!d->elideConnections.contains(quickItem)) {
-                // add "refresh" elide state connections which remove themself
-                auto clearElideInformation = [this, quickItem]() {
-                    //quickItem->setColor(originColor);
-                    for (QMetaObject::Connection connection : d->elideConnections.value(quickItem))
-                        quickItem->disconnect(connection);
-                    d->elideConnections.remove(quickItem);
-                };
-
-                auto connectWithChangedWidthThreshold = [=] () {
-                    return connect(quickItem, &QQuickItem::widthChanged, [=]() {
-                        if (quickItem->implicitWidth() <= quickItem->width())
-                            clearElideInformation();
-                    });
-                };
-                auto connectImplicitWidthChangedThreshold = [=] () {
-                    return connect(quickItem, &QQuickItem::implicitWidthChanged, [=]() {
-                        if (quickItem->implicitWidth() <= quickItem->width())
-                            clearElideInformation();
-                    });
-                };
-
-                d->elideConnections.insert(quickItem,
-                                          {connectWithChangedWidthThreshold(),
-                                           connectImplicitWidthChangedThreshold()});
-            }
-        }
-    }
-    return elidedTextResult;
-}
-
 void QQmlDebugTranslationServiceImpl::foundTranslationBinding(const TranslationBindingInformation &translationBindingInformation)
 {
     QObject *scopeObject = translationBindingInformation.scopeObject;

src/plugins/qmltooling/qmldbg_preview/qqmldebugtranslationservice.h

@@ -68,7 +68,6 @@ public:
     QQmlDebugTranslationServiceImpl(QObject *parent = 0);
     ~QQmlDebugTranslationServiceImpl();
 
-    QString foundElidedText(QObject *textObject, const QString &layoutText, const QString &elideText) override;
     void foundTranslationBinding(const TranslationBindingInformation &translationBindingInformation) override;
 
     void messageReceived(const QByteArray &message) override;
@@ -80,7 +79,8 @@ signals:
     void state(const QString &stateName);
     void stateList();
     void watchTextElides(bool);
-    void missingTranslations();
+    void translationIssues();
+    void elidedTranslations();
     void sendTranslatableTextOccurrences();
 
 private:

src/qml/debugger/qqmldebugserviceinterfaces_p.h

@@ -108,7 +108,6 @@ class QQmlEngineControlService {};
 class QQmlNativeDebugService {};
 class QQmlDebugTranslationService {
 public:
-    virtual QString foundElidedText(QObject *, const QString &, const QString &) {return {};}
     virtual void foundTranslationBinding(const TranslationBindingInformation &) {}
 };
 
@@ -186,7 +185,6 @@ class Q_QML_PRIVATE_EXPORT QQmlDebugTranslationService : public QQmlDebugService
 public:
     static const QString s_key;
 
-    virtual QString foundElidedText(QObject *qQuickTextObject, const QString &layoutText, const QString &elideText) = 0;
     virtual void foundTranslationBinding(const TranslationBindingInformation &translationBindingInformation) = 0;
 protected:
     friend class QQmlDebugConnector;

src/qml/debugger/qqmldebugtranslationprotocol_p.h

@@ -64,19 +64,23 @@ enum class Request {
     ChangeLanguage = 1,
     StateList,
     ChangeState,
-    MissingTranslations,
+    TranslationIssues,
     TranslatableTextOccurrences,
     WatchTextElides,
     DisableWatchTextElides,
+    // following are obsolete, just provided for compilation compatibility
+    MissingTranslations
 };
 
 enum class Reply {
     LanguageChanged = 101,
     StateList,
     StateChanged,
-    MissingTranslations,
+    TranslationIssues,
     TranslatableTextOccurrences,
-    TextElided,
+    // following are obsolete, just provided for compilation compatibility
+    MissingTranslations,
+    TextElided
 };
 
 inline QDataStream &operator<<(QDataStream &ds, Request r)
@@ -124,6 +128,12 @@ inline QByteArray createMissingTranslationsRequest(QDataStream &packet)
     return qobject_cast<QBuffer *>(packet.device())->data();
 }
 
+inline QByteArray createTranslationIssuesRequest(QDataStream &packet)
+{
+    packet << Request::TranslationIssues;
+    return qobject_cast<QBuffer *>(packet.device())->data();
+}
+
 inline QByteArray createTranslatableTextOccurrencesRequest(QDataStream &packet)
 {
     packet << Request::TranslatableTextOccurrences;

src/qmldebug/qqmldebugtranslationclient.cpp

@@ -59,7 +59,7 @@ void QQmlDebugTranslationClient::messageReceived(const QByteArray &message)
 
     packet >> type;
     switch (type) {
-    case QQmlDebugTranslation::Reply::MissingTranslations: {
+    case QQmlDebugTranslation::Reply::TranslationIssues: {
         packet >> translationIssues;
         break;
     }

src/quick/items/qquicktext.cpp

@@ -1159,12 +1159,6 @@ QRectF QQuickTextPrivate::setupTextLayout(qreal *const baseline)
 
         elideLayout->setFont(layout.font());
         elideLayout->setTextOption(layout.textOption());
-#if QT_CONFIG(translation) && QT_CONFIG(qml_debug)
-        if (QQmlDebugTranslationService *service
-                     = QQmlDebugConnector::service<QQmlDebugTranslationService>()) {
-            elideText = service->foundElidedText(q, layoutText, elideText);
-        }
-#endif //QT_CONFIG(translation)
         elideLayout->setText(elideText);
         elideLayout->beginLayout();
 

tests/auto/qml/debugger/qqmldebugtranslationclient/tst_qqmldebugtranslationclient.cpp

@@ -75,7 +75,7 @@ private slots:
     {
         QVersionedPacket<QQmlDebugConnector> packet;
         m_debugTranslationClient->sendMessage(
-                QQmlDebugTranslation::createMissingTranslationsRequest(packet));
+                QQmlDebugTranslation::createTranslationIssuesRequest(packet));
 
         QTRY_VERIFY(m_debugTranslationClient->translationIssues.size() > 0);
     }

tests/auto/qml/debugger/qqmldebugtranslationservice/data/test.qml

@@ -30,7 +30,7 @@ import QtQuick
 
 Rectangle {
     id: root
-    width: 200
+    width: 130
     height: 200
     property int widthFactor: 7
 
@@ -68,15 +68,6 @@ Rectangle {
         }
     }
 
-    // this is necessary to have the test working for different font sizes and dpi settings
-    Text {
-        id: originHelloTextToGetTheNecessaryWidth
-        text: "short"
-        opacity: 0
-        anchors.bottom: root.bottom
-        onWidthChanged: root.width = originHelloTextToGetTheNecessaryWidth.width * widthFactor
-    }
-
     states: [
         State {
             name: "BiggerFontState"
@@ -96,10 +87,6 @@ Rectangle {
                 font.pointSize: 20
             }
 
-            PropertyChanges {
-                target: originHelloTextToGetTheNecessaryWidth
-                font.pointSize: 20
-            }
         },
         State {
             name: "WayBiggerFontState"
@@ -118,11 +105,6 @@ Rectangle {
                 target: text3
                 font.pointSize: 30
             }
-
-            PropertyChanges {
-                target: originHelloTextToGetTheNecessaryWidth
-                font.pointSize: 30
-            }
         }
     ]
 }

tests/auto/qml/debugger/qqmldebugtranslationservice/tst_qqmldebugtranslationservice.cpp

@@ -84,24 +84,23 @@ private slots:
         QVERIFY(currentDebugServiceMessage().isEmpty());
     }
 
-
     void verifyMissingAllTranslationsForMissingLanguage()
     {
         changeLanguage("ru");
-        auto missingTranslations = getMissingTranslations();
+        auto translationIssues = getTranslationIssues();
 
-        QCOMPARE(missingTranslations.length(), getTranslatableTextOccurrences().count());
+        QCOMPARE(translationIssues.length(), getTranslatableTextOccurrences().count());
-        QCOMPARE(missingTranslations.at(0).language, "ru ru-RU ru-Cyrl-RU");
+        QCOMPARE(translationIssues.at(0).language, "ru ru-RU ru-Cyrl-RU");
     }
 
     void verifyCorrectNumberOfMissingTranslations()
     {
         changeLanguage("fr");
 
-        auto missingTranslations = getMissingTranslations();
+        auto translationIssues = getTranslationIssues();
 
-        QCOMPARE(missingTranslations.length(), 3);
+        QCOMPARE(translationIssues.length(), 3);
-        QCOMPARE(missingTranslations.at(0).language, "fr fr-FR fr-Latn-FR");
+        QCOMPARE(translationIssues.at(0).language, "fr fr-FR fr-Latn-FR");
     }
 
     void verifyCorrectNumberOfTranslatableTextOccurrences()
@@ -114,6 +113,43 @@ private slots:
         QCOMPARE(getStates().length(), 2);
     }
 
+    void getElideWarnings()
+    {
+        QVersionedPacket<QQmlDebugConnector> packet;
+        sendMessageToService(createWatchTextElidesRequest(packet));
+
+        changeLanguage("es");
+        auto translationIssues = getTranslationIssues();
+
+        int elideWarningCount = 0;
+        for (auto issue : translationIssues) {
+            if (issue.type == TranslationIssue::Type::Elided) {
+                elideWarningCount++;
+            }
+        }
+        QCOMPARE(elideWarningCount, 1);
+    }
+
+    void getElideWarningsWhenStateChanged()
+    {
+        QVersionedPacket<QQmlDebugConnector> packet;
+        sendMessageToService(createWatchTextElidesRequest(packet));
+
+        changeLanguage("es");
+
+        sendMessageToService(createChangeStateRequest(packet, "WayBiggerFontState"));
+
+        auto translationIssues = getTranslationIssues();
+
+        int elideWarningCount = 0;
+        for (auto issue : translationIssues) {
+            if (issue.type == TranslationIssue::Type::Elided) {
+                elideWarningCount++;
+            }
+        }
+        QCOMPARE(elideWarningCount, 1);
+    }
+
     void loopThroughAllStates()
     {
         QVector<QmlState> stateList = getStates();
@@ -137,59 +173,6 @@ private slots:
         }
     }
 
-    void getElideWarnings()
-    {
-        QVersionedPacket<QQmlDebugConnector> packet;
-        sendMessageToService(createWatchTextElidesRequest(packet));
-
-        changeLanguage("fr");
-
-        // after language changes, we get elide warnings
-        auto replies = currentReply();
-        int elideCount = 0;
-
-        for (auto reply : replies) {
-            QVersionedPacket<QQmlDebugConnector> readPacket(reply);
-            TranslationIssue issue;
-            Reply replyType;
-
-            readPacket >> replyType;
-            if (replyType == Reply::TextElided) {
-                readPacket >> issue;
-                QCOMPARE(issue.codeMarker.line, 47);
-                QCOMPARE(issue.language, "fr fr-FR fr-Latn-FR");
-                elideCount++;
-            }
-        }
-    }
-
-    void getElideWarningsWhenStateChanged()
-    {
-        QVersionedPacket<QQmlDebugConnector> packet;
-        sendMessageToService(createWatchTextElidesRequest(packet));
-
-        changeLanguage("fr");
-
-        const QString stateName("BiggerFontState");
-        sendMessageToService(createChangeStateRequest(packet, stateName));
-
-        auto replies = currentReply();
-
-        int elideCount = 0;
-        for (auto reply : replies) {
-            QVersionedPacket<QQmlDebugConnector> readPacket(reply);
-            TranslationIssue issue;
-            Reply replyType;
-
-            readPacket >> replyType;
-            if (replyType == Reply::TextElided) {
-                readPacket >> issue;
-                QCOMPARE(issue.codeMarker.line, 47);
-                elideCount++;
-            }
-        }
-    }
-
 private:
 
     QVector<QmlElement> getTranslatableTextOccurrences()
@@ -231,18 +214,18 @@ private:
         // QTest::qWait(500);
     }
 
-    QVector<TranslationIssue> getMissingTranslations()
+    QVector<TranslationIssue> getTranslationIssues()
     {
         QVersionedPacket<QQmlDebugConnector> packet;
-        sendMessageToService(createMissingTranslationsRequest(packet));
+        sendMessageToService(createTranslationIssuesRequest(packet));
         QVersionedPacket<QQmlDebugConnector> readPacket(currentReply().at(0));
 
         Reply replyType;
-        QVector<TranslationIssue> missingTranslations;
+        QVector<TranslationIssue> translationIssues;
         readPacket >> replyType;
-        readPacket >> missingTranslations;
+        readPacket >> translationIssues;
 
-        return missingTranslations;
+        return translationIssues;
     }
 
     QByteArray debugServiceMessage(const QByteArray &data)
@@ -323,12 +306,10 @@ private:
                 return "LanguageChanged";
             case Reply::StateChanged:
                 return "StateChanged";
-            case Reply::MissingTranslations:
+            case Reply::TranslationIssues:
-                return "MissingTranslations";
+                return "TranslationIssues";
             case Reply::TranslatableTextOccurrences:
                 return "TranslatableTextOccurrences";
-            case Reply::TextElided:
-                return "TextElided";
             default:
                 Q_ASSERT_X(false, "not implemented", "not implemented");
             }
@@ -341,7 +322,7 @@ private:
             QVersionedPacket<QQmlDebugConnector> readPacket(message);
             readPacket >> replyType;
             debugString.append(replyTypeToString(replyType));
-            if (replyType == Reply::MissingTranslations) {
+            if (replyType == Reply::TranslationIssues) {
                 QVector<TranslationIssue> translationIssues;
                 readPacket >> translationIssues;
                 QStringList translationIssueStrings;
@@ -356,11 +337,6 @@ private:
                 debugString.append(translationIssuesString.arg(QString::number(translationIssues.size()),
                                                                translationIssueStrings.join("; ")));
             }
-            if (replyType == Reply::TextElided) {
-                TranslationIssue translationIssue;
-                readPacket >> translationIssue;
-                debugString.append(QString(" %1 ").arg(translationIssue.toDebugString()));
-            }
         }
         return debugString;
     }