Correctly ignore unknown tags in StyledText

Also improve tests to compare expected formatting and improve coverage.

Change-Id: I021dbdcd147dd7340a0dc1c30f4b104f22efece0
Reviewed-by: Yann Bodson <yann.bodson@nokia.com>

diff --git a/src/quick/util/qdeclarativestyledtext.cpp b/src/quick/util/qdeclarativestyledtext.cpp
index 2f79fad..d2c2ef8 100644 (file)
--- a/src/quick/util/qdeclarativestyledtext.cpp
+++ b/src/quick/util/qdeclarativestyledtext.cpp
@@ -160,6 +160,7 @@ void QDeclarativeStyledTextPrivate::parse()
     int textStart = 0;
     int textLength = 0;
     int rangeStart = 0;
+    bool formatChanged = false;
     const QChar *ch = text.constData();
     while (!ch->isNull()) {
         if (*ch == lessThan) {
@@ -177,26 +178,35 @@ void QDeclarativeStyledTextPrivate::parse()
                 }
             }
             if (rangeStart != drawText.length() && formatStack.count()) {
-                QTextLayout::FormatRange formatRange;
-                formatRange.format = formatStack.top();
-                formatRange.start = rangeStart;
-                formatRange.length = drawText.length() - rangeStart;
-                ranges.append(formatRange);
+                if (formatChanged) {
+                    QTextLayout::FormatRange formatRange;
+                    formatRange.format = formatStack.top();
+                    formatRange.start = rangeStart;
+                    formatRange.length = drawText.length() - rangeStart;
+                    ranges.append(formatRange);
+                    formatChanged = false;
+                } else if (ranges.count()) {
+                    ranges.last().length += drawText.length() - rangeStart;
+                }
             }
             rangeStart = drawText.length();
             ++ch;
             if (*ch == slash) {
                 ++ch;
                 if (parseCloseTag(ch, text, drawText)) {
-                    if (formatStack.count())
+                    if (formatStack.count()) {
+                        formatChanged = true;
                         formatStack.pop();
+                    }
                 }
             } else {
                 QTextCharFormat format;
                 if (formatStack.count())
                     format = formatStack.top();
-                if (parseTag(ch, text, drawText, format))
+                if (parseTag(ch, text, drawText, format)) {
+                    formatChanged = true;
                     formatStack.push(format);
+                }
             }
             textStart = ch - text.constData() + 1;
             textLength = 0;
@@ -215,11 +225,15 @@ void QDeclarativeStyledTextPrivate::parse()
     if (textLength)
         drawText.append(QStringRef(&text, textStart, textLength));
     if (rangeStart != drawText.length() && formatStack.count()) {
-        QTextLayout::FormatRange formatRange;
-        formatRange.format = formatStack.top();
-        formatRange.start = rangeStart;
-        formatRange.length = drawText.length() - rangeStart;
-        ranges.append(formatRange);
+        if (formatChanged) {
+            QTextLayout::FormatRange formatRange;
+            formatRange.format = formatStack.top();
+            formatRange.start = rangeStart;
+            formatRange.length = drawText.length() - rangeStart;
+            ranges.append(formatRange);
+        } else if (ranges.count()) {
+            ranges.last().length += drawText.length() - rangeStart;
+        }
     }
 
     layout.setText(drawText);
@@ -239,27 +253,28 @@ bool QDeclarativeStyledTextPrivate::parseTag(const QChar *&ch, const QString &te
             QStringRef tag(&textIn, tagStart, tagLength);
             const QChar char0 = tag.at(0);
             if (char0 == QLatin1Char('b')) {
-                if (tagLength == 1)
+                if (tagLength == 1) {
                     format.setFontWeight(QFont::Bold);
-                else if (tagLength == 2 && tag.at(1) == QLatin1Char('r')) {
+                    return true;
+                } else if (tagLength == 2 && tag.at(1) == QLatin1Char('r')) {
                     textOut.append(QChar(QChar::LineSeparator));
                     return false;
                 }
             } else if (char0 == QLatin1Char('i')) {
-                if (tagLength == 1)
+                if (tagLength == 1) {
                     format.setFontItalic(true);
+                    return true;
+                }
             } else if (char0 == QLatin1Char('p')) {
                 if (tagLength == 1) {
                     if (!hasNewLine)
                         textOut.append(QChar::LineSeparator);
                 }
-            } else if (char0 == QLatin1Char('s')) {
-                if (tag == QLatin1String("strong"))
-                    format.setFontWeight(QFont::Bold);
             } else if (char0 == QLatin1Char('u')) {
-                if (tagLength == 1)
+                if (tagLength == 1) {
                     format.setFontUnderline(true);
-                else if (tag == QLatin1String("ul")) {
+                    return true;
+                } else if (tag == QLatin1String("ul")) {
                     List listItem;
                     listItem.level = 0;
                     listItem.type = Unordered;
@@ -274,7 +289,11 @@ bool QDeclarativeStyledTextPrivate::parseTag(const QChar *&ch, const QString &te
                         textOut.append(QChar::LineSeparator);
                     format.setFontPointSize(baseFont.pointSize() * scaling[level - 1]);
                     format.setFontWeight(QFont::Bold);
+                    return true;
                 }
+            } else if (tag == QLatin1String("strong")) {
+                format.setFontWeight(QFont::Bold);
+                return true;
             } else if (tag == QLatin1String("ol")) {
                 List listItem;
                 listItem.level = 0;
@@ -317,16 +336,20 @@ bool QDeclarativeStyledTextPrivate::parseTag(const QChar *&ch, const QString &te
                     textOut += QString(2, QChar::Nbsp);
                 }
             }
-            return true;
+            return false;
         } else if (ch->isSpace()) {
             // may have params.
             QStringRef tag(&textIn, tagStart, tagLength);
             if (tag == QLatin1String("font"))
                 return parseFontAttributes(ch, textIn, format);
-            if (tag == QLatin1String("ol"))
-                return parseOrderedListAttributes(ch, textIn);
-            if (tag == QLatin1String("ul"))
-                return parseUnorderedListAttributes(ch, textIn);
+            if (tag == QLatin1String("ol")) {
+                parseOrderedListAttributes(ch, textIn);
+                return false; // doesn't modify format
+            }
+            if (tag == QLatin1String("ul")) {
+                parseUnorderedListAttributes(ch, textIn);
+                return false; // doesn't modify format
+            }
             if (tag == QLatin1String("a")) {
                 return parseAnchorAttributes(ch, textIn, format);
             }
@@ -357,7 +380,7 @@ bool QDeclarativeStyledTextPrivate::parseCloseTag(const QChar *&ch, const QStrin
                 if (tagLength == 1)
                     return true;
                 else if (tag.at(1) == QLatin1Char('r') && tagLength == 2)
-                    return true;
+                    return false;
             } else if (char0 == QLatin1Char('i')) {
                 if (tagLength == 1)
                     return true;
@@ -368,7 +391,7 @@ bool QDeclarativeStyledTextPrivate::parseCloseTag(const QChar *&ch, const QStrin
                 if (tagLength == 1) {
                     textOut.append(QChar::LineSeparator);
                     hasNewLine = true;
-                    return true;
+                    return false;
                 }
             } else if (char0 == QLatin1Char('u')) {
                 if (tagLength == 1)
@@ -379,7 +402,7 @@ bool QDeclarativeStyledTextPrivate::parseCloseTag(const QChar *&ch, const QStrin
                         if (!listStack.count())
                             textOut.append(QChar::LineSeparator);
                     }
-                    return true;
+                    return false;
                 }
             } else if (char0 == QLatin1Char('h') && tagLength == 2) {
                 textOut.append(QChar::LineSeparator);
@@ -395,9 +418,9 @@ bool QDeclarativeStyledTextPrivate::parseCloseTag(const QChar *&ch, const QStrin
                     if (!listStack.count())
                         textOut.append(QChar::LineSeparator);
                 }
-                return true;
+                return false;
             } else if (tag == QLatin1String("li")) {
-                return true;
+                return false;
             }
             return false;
         } else if (!ch->isSpace()){

diff --git a/tests/auto/qtquick2/qdeclarativestyledtext/tst_qdeclarativestyledtext.cpp b/tests/auto/qtquick2/qdeclarativestyledtext/tst_qdeclarativestyledtext.cpp
index 2c5c66f..a17c429 100644 (file)
--- a/tests/auto/qtquick2/qdeclarativestyledtext/tst_qdeclarativestyledtext.cpp
+++ b/tests/auto/qtquick2/qdeclarativestyledtext/tst_qdeclarativestyledtext.cpp
@@ -51,49 +51,95 @@ public:
     {
     }
 
+    struct Format {
+        enum Type {
+            Bold = 0x01,
+            Underline = 0x02,
+            Italic = 0x04
+        };
+        Format(int t, int s, int l)
+            : type(t), start(s), length(l) {}
+        int type;
+        int start;
+        int length;
+    };
+    typedef QList<Format> FormatList;
+
 private slots:
     void textOutput();
     void textOutput_data();
 };
 
-// For malformed input all we test is that we get the expected text out.
+Q_DECLARE_METATYPE(tst_qdeclarativestyledtext::FormatList);
+
+
+// For malformed input all we test is that we get the expected text and format out.
 // 
 void tst_qdeclarativestyledtext::textOutput_data()
 {
     QTest::addColumn<QString>("input");
     QTest::addColumn<QString>("output");
+    QTest::addColumn<FormatList>("formats");
 
-    QTest::newRow("bold") << "<b>bold</b>" << "bold";
-    QTest::newRow("italic") << "<b>italic</b>" << "italic";
-    QTest::newRow("strong") << "<strong>strong</strong>" << "strong";
-    QTest::newRow("missing >") << "<b>text</b" << "text";
-    QTest::newRow("missing b>") << "<b>text</" << "text";
-    QTest::newRow("missing /b>") << "<b>text<" << "text";
-    QTest::newRow("missing </b>") << "<b>text" << "text";
-    QTest::newRow("bad nest") << "<b>text <i>italic</b></i>" << "text italic";
-    QTest::newRow("font color") << "<font color=\"red\">red text</font>" << "red text";
-    QTest::newRow("font color: single quote") << "<font color='red'>red text</font>" << "red text";
-    QTest::newRow("font size") << "<font size=\"1\">text</font>" << "text";
-    QTest::newRow("font empty") << "<font>text</font>" << "text";
-    QTest::newRow("font bad 1") << "<font ezis=\"blah\">text</font>" << "text";
-    QTest::newRow("font bad 2") << "<font size=\"1>text</font>" << "";
-    QTest::newRow("extra close") << "<b>text</b></b>" << "text";
-    QTest::newRow("extra space") << "<b >text</b>" << "text";
-    QTest::newRow("entities") << "&lt;b&gt;this &amp; that&lt;/b&gt;" << "<b>this & that</b>";
-    QTest::newRow("newline") << "text<br>more text" << QLatin1String("text") + QChar(QChar::LineSeparator) + QLatin1String("more text")  ;
-    QTest::newRow("self-closing newline") << "text<br/>more text" << QLatin1String("text") + QChar(QChar::LineSeparator) + QLatin1String("more text")  ;
-    QTest::newRow("empty") << "" << "";
+    QTest::newRow("bold") << "<b>bold</b>" << "bold" << (FormatList() << Format(Format::Bold, 0, 4));
+    QTest::newRow("italic") << "<i>italic</i>" << "italic" << (FormatList() << Format(Format::Italic, 0, 6));
+    QTest::newRow("strong") << "<strong>strong</strong>" << "strong" << (FormatList() << Format(Format::Bold, 0, 6));
+    QTest::newRow("missing >") << "<b>text</b" << "text" << (FormatList() << Format(Format::Bold, 0, 4));
+    QTest::newRow("missing b>") << "<b>text</" << "text" << (FormatList() << Format(Format::Bold, 0, 4));
+    QTest::newRow("missing /b>") << "<b>text<" << "text" << (FormatList() << Format(Format::Bold, 0, 4));
+    QTest::newRow("missing </b>") << "<b>text" << "text" << (FormatList() << Format(Format::Bold, 0, 4));
+    QTest::newRow("nested") << "<b>text <i>italic</i> bold</b>" << "text italic bold" << (FormatList() << Format(Format::Bold, 0, 5) << Format(Format::Bold | Format::Italic, 5, 6) << Format(Format::Bold, 11, 5));
+    QTest::newRow("bad nest") << "<b>text <i>italic</b></i>" << "text italic" << (FormatList() << Format(Format::Bold, 0, 5) << Format(Format::Bold | Format::Italic, 5, 6));
+    QTest::newRow("font color") << "<font color=\"red\">red text</font>" << "red text" << (FormatList() << Format(0, 0, 8));
+    QTest::newRow("font color: single quote") << "<font color='red'>red text</font>" << "red text" << (FormatList() << Format(0, 0, 8));
+    QTest::newRow("font size") << "<font size=\"1\">text</font>" << "text" << (FormatList() << Format(0, 0, 4));
+    QTest::newRow("font empty") << "<font>text</font>" << "text" << FormatList();
+    QTest::newRow("font bad 1") << "<font ezis=\"blah\">text</font>" << "text" << FormatList();
+    QTest::newRow("font bad 2") << "<font size=\"1>text</font>" << "" << FormatList();
+    QTest::newRow("extra close") << "<b>text</b></b>" << "text" << (FormatList() << Format(Format::Bold, 0, 4));
+    QTest::newRow("extra space") << "<b >text</b>" << "text" << (FormatList() << Format(Format::Bold, 0, 4));
+    QTest::newRow("entities") << "&lt;b&gt;this &amp; that&lt;/b&gt;" << "<b>this & that</b>" << FormatList();
+    QTest::newRow("newline") << "text<br>more text" << QLatin1String("text") + QChar(QChar::LineSeparator) + QLatin1String("more text") << FormatList();
+    QTest::newRow("paragraph") << "text<p>more text" << QLatin1String("text") + QChar(QChar::LineSeparator) + QLatin1String("more text") << FormatList();
+    QTest::newRow("paragraph closed") << "text<p>more text</p>more text" << QLatin1String("text") + QChar(QChar::LineSeparator) + QLatin1String("more text")  + QChar(QChar::LineSeparator) + QLatin1String("more text") << FormatList();
+    QTest::newRow("paragraph closed bold") << "<b>text<p>more text</p>more text</b>" << QLatin1String("text") + QChar(QChar::LineSeparator) + QLatin1String("more text")  + QChar(QChar::LineSeparator) + QLatin1String("more text") << (FormatList() << Format(Format::Bold, 0, 24));
+    QTest::newRow("self-closing newline") << "text<br/>more text" << QLatin1String("text") + QChar(QChar::LineSeparator) + QLatin1String("more text") << FormatList();
+    QTest::newRow("empty") << "" << "" << FormatList();
+    QTest::newRow("unknown tag") << "<a href='#'><foo>underline</foo></a> not" << "underline not" << (FormatList() << Format(Format::Underline, 0, 9));
+    QTest::newRow("h0") << "<h0>head" << "head" << FormatList();
+    QTest::newRow("h1") << "<h1>head" << QChar(QChar::LineSeparator) + QLatin1String("head") << (FormatList() << Format(Format::Bold, 0, 5));
+    QTest::newRow("h2") << "<h2>head" << QChar(QChar::LineSeparator) + QLatin1String("head") << (FormatList() << Format(Format::Bold, 0, 5));
+    QTest::newRow("h3") << "<h3>head" << QChar(QChar::LineSeparator) + QLatin1String("head") << (FormatList() << Format(Format::Bold, 0, 5));
+    QTest::newRow("h4") << "<h4>head" << QChar(QChar::LineSeparator) + QLatin1String("head") << (FormatList() << Format(Format::Bold, 0, 5));
+    QTest::newRow("h5") << "<h5>head" << QChar(QChar::LineSeparator) + QLatin1String("head") << (FormatList() << Format(Format::Bold, 0, 5));
+    QTest::newRow("h6") << "<h6>head" << QChar(QChar::LineSeparator) + QLatin1String("head") << (FormatList() << Format(Format::Bold, 0, 5));
+    QTest::newRow("h7") << "<h7>head" << "head" << FormatList();
 }
 
 void tst_qdeclarativestyledtext::textOutput()
 {
     QFETCH(QString, input);
     QFETCH(QString, output);
+    QFETCH(FormatList, formats);
 
     QTextLayout layout;
     QDeclarativeStyledText::parse(input, layout);
 
     QCOMPARE(layout.text(), output);
+
+    QList<QTextLayout::FormatRange> layoutFormats = layout.additionalFormats();
+
+    QCOMPARE(layoutFormats.count(), formats.count());
+    for (int i = 0; i < formats.count(); ++i) {
+        QCOMPARE(layoutFormats.at(i).start, formats.at(i).start);
+        QCOMPARE(layoutFormats.at(i).length, formats.at(i).length);
+        if (formats.at(i).type & Format::Bold)
+            QVERIFY(layoutFormats.at(i).format.fontWeight() == QFont::Bold);
+        else
+            QVERIFY(layoutFormats.at(i).format.fontWeight() == QFont::Normal);
+        QVERIFY(layoutFormats.at(i).format.fontItalic() == bool(formats.at(i).type & Format::Italic));
+        QVERIFY(layoutFormats.at(i).format.fontUnderline() == bool(formats.at(i).type & Format::Underline));
+    }
 }