From ca64365e3a7245f6ef0f8f6962db3c1b85421cef Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Mon, 12 Dec 2022 11:04:54 +0100 Subject: [PATCH] Don't hide object replacement char except in rich text The object replacement character (U+FFFC) is used to represent inline objects such as images in rich-text. To enable this, we have special handling of it in QTextEngine. For classes where inline images are not supported, it will just be hidden from the visual text, which is unexpected. Instead of always special-casing it, we make this dependent on whether the document layout has registered any object handlers. If they have not, then there will be no visual representation of the object, and it is better to show the glyph for it. For anything based on QTextDocument, there will always be the image handler, so U+FFFC will still have special handling there, but for non-rich labels and plain text editors the glyph will be shown instead. Note that there was also a bug in QLineEdit, where the object replacement character was always replaced by a space. This was introduced in 2007, in a patch which replaced a !ch.isPrint() with a check for "the most obvious non-printable characters" to reduce the number of characters that were not shown. However, U+FFFC is a printable character and would thus not have been filtered by the !isPrint() condition, so I think this was a mistake at the time. However, due to the special-casing of the character in Qt, it would not have had any effect until now. This also changes the QTextLayout::cursorToXForInlineObject() test to actually test proper inline objects, as this was previously using a hack which depended on the inline object code to be used even for plain QTextLayouts with no handlers for these. [ChangeLog][Text] The object replacement character (U+FFFC) is now only filtered out in rich text controls, where they represent inline objects. In other controls, its glyphs will be shown as with other text. Pick-to: 6.5 Fixes: QTBUG-101526 Change-Id: I7fcaf2b10918feb41589e1098016efbf79a0e62d Reviewed-by: Lars Knoll Reviewed-by: Qt CI Bot --- src/gui/text/qabstracttextdocumentlayout_p.h | 11 ++++++++ src/gui/text/qtextengine.cpp | 13 +++++++++- src/widgets/widgets/qwidgetlinecontrol.cpp | 3 +-- .../auto/gui/text/qglyphrun/tst_qglyphrun.cpp | 16 ++++++++++++ .../gui/text/qtextlayout/tst_qtextlayout.cpp | 24 +++++++++++------- tests/auto/shared/resources/test.ttf | Bin 2128 -> 2196 bytes 6 files changed, 55 insertions(+), 12 deletions(-) diff --git a/src/gui/text/qabstracttextdocumentlayout_p.h b/src/gui/text/qabstracttextdocumentlayout_p.h index 34eccd89afa..f10c1d9bd20 100644 --- a/src/gui/text/qabstracttextdocumentlayout_p.h +++ b/src/gui/text/qabstracttextdocumentlayout_p.h @@ -18,6 +18,7 @@ #include #include "private/qobject_p.h" #include "qtextdocument_p.h" +#include "qabstracttextdocumentlayout.h" #include "QtCore/qhash.h" QT_BEGIN_NAMESPACE @@ -46,6 +47,16 @@ public: docPrivate = QTextDocumentPrivate::get(doc); } + static QAbstractTextDocumentLayoutPrivate *get(QAbstractTextDocumentLayout *layout) + { + return layout->d_func(); + } + + bool hasHandlers() const + { + return !handlers.isEmpty(); + } + inline int _q_dynamicPageCountSlot() const { return q_func()->pageCount(); } inline QSizeF _q_dynamicDocumentSizeSlot() const diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp index c174eefa70e..eded3e3f334 100644 --- a/src/gui/text/qtextengine.cpp +++ b/src/gui/text/qtextengine.cpp @@ -7,6 +7,7 @@ #include "qtextformat_p.h" #include "qtextengine_p.h" #include "qabstracttextdocumentlayout.h" +#include "qabstracttextdocumentlayout_p.h" #include "qtextlayout.h" #include "qtextboundaryfinder.h" #include @@ -1933,7 +1934,17 @@ void QTextEngine::itemize() const while (uc < e) { switch (*uc) { case QChar::ObjectReplacementCharacter: - analysis->flags = QScriptAnalysis::Object; + { + const QTextDocumentPrivate *doc_p = QTextDocumentPrivate::get(block); + if (doc_p != nullptr + && doc_p->layout() != nullptr + && QAbstractTextDocumentLayoutPrivate::get(doc_p->layout()) != nullptr + && QAbstractTextDocumentLayoutPrivate::get(doc_p->layout())->hasHandlers()) { + analysis->flags = QScriptAnalysis::Object; + } else { + analysis->flags = QScriptAnalysis::None; + } + } break; case QChar::LineSeparator: analysis->flags = QScriptAnalysis::LineOrParagraphSeparator; diff --git a/src/widgets/widgets/qwidgetlinecontrol.cpp b/src/widgets/widgets/qwidgetlinecontrol.cpp index 30465b95fa7..ed7bc035ec3 100644 --- a/src/widgets/widgets/qwidgetlinecontrol.cpp +++ b/src/widgets/widgets/qwidgetlinecontrol.cpp @@ -85,8 +85,7 @@ void QWidgetLineControl::updateDisplayText(bool forceUpdate) for (int i = 0; i < (int)str.size(); ++i) { if ((uc[i].unicode() < 0x20 && uc[i].unicode() != 0x09) || uc[i] == QChar::LineSeparator - || uc[i] == QChar::ParagraphSeparator - || uc[i] == QChar::ObjectReplacementCharacter) + || uc[i] == QChar::ParagraphSeparator) uc[i] = QChar(0x0020); } diff --git a/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp b/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp index efd590a730a..9660d829b8d 100644 --- a/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp +++ b/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp @@ -42,6 +42,7 @@ private slots: void stringIndexes(); void retrievalFlags_data(); void retrievalFlags(); + void objectReplacementCharacter(); private: int m_testFontId; @@ -954,6 +955,21 @@ void tst_QGlyphRun::retrievalFlags() QCOMPARE(firstGlyphRun.positions().isEmpty(), !expectedGlyphPositions); } +void tst_QGlyphRun::objectReplacementCharacter() +{ + QTextLayout layout; + layout.setFont(m_testFont); + layout.setText(QStringLiteral("\uFFFC")); + layout.beginLayout(); + layout.createLine(); + layout.endLayout(); + + QList glyphRuns = layout.glyphRuns(); + QCOMPARE(glyphRuns.size(), 1); + QCOMPARE(glyphRuns.first().glyphIndexes().size(), 1); + QCOMPARE(glyphRuns.first().glyphIndexes().first(), 5); +} + #endif // QT_NO_RAWFONT QTEST_MAIN(tst_QGlyphRun) diff --git a/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp b/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp index 3958e2a01c9..9e6daf81b13 100644 --- a/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp +++ b/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp @@ -512,18 +512,24 @@ void tst_QTextLayout::noWrap() void tst_QTextLayout::cursorToXForInlineObjects() { - QChar ch(QChar::ObjectReplacementCharacter); - QString text(ch); - QTextLayout layout(text, testFont); - layout.beginLayout(); + QString text = QStringLiteral(""); - QTextEngine *engine = layout.engine(); - const int item = engine->findItem(0); - engine->layoutData->items[item].width = 32; + QTextDocument document; + document.setHtml(text); + QCOMPARE(document.blockCount(), 1); - QTextLine line = layout.createLine(); - line.setLineWidth(0x10000); + // Trigger layout + { + QImage img(1, 1, QImage::Format_ARGB32_Premultiplied); + QPainter p(&img); + document.drawContents(&p); + } + QTextLayout *layout = document.firstBlock().layout(); + QVERIFY(layout != nullptr); + QCOMPARE(layout->lineCount(), 1); + + QTextLine line = layout->lineAt(0); QCOMPARE(line.cursorToX(0), qreal(0)); QCOMPARE(line.cursorToX(1), qreal(32)); } diff --git a/tests/auto/shared/resources/test.ttf b/tests/auto/shared/resources/test.ttf index b2bb6cd036eae9b4b127f88d018e7f30106787f6..8b56c046d4e06ee0292d8e5a82fb284aeae38fd6 100644 GIT binary patch delta 587 zcmYjNOK1~O6g~INBx$5I2HSvGA+dF1t4_vRMQf}yWNWJk6$)nZX%p>ahIT5g2vI@E z!d1tWWaG|77XeXRDCo|W6qh2l%kEtHS(J=tN*8@_-o5vnd(Y#(%_A?8O$dNKJcfqp z8@JAxPp#+o0GTCQEZdg5T{)R$y-AeH54<$`sX*W0rRLoQJ9?*m5zxk{FE1>v&6j?@ zic|kY`BANERpRwuAK3eY7_X6t)TGJY&}F!0d(G8DiJ(a0J&Dw!Q?|5%+FS z>Z+7@)Gx74H!QoF_Ar0QyFozqKoxuoK5zX^3*zD2o0NK~bjdO6=rd|Wk#0##c4!*|PGyoA zEtyG5tNTQnz1?<8irtRnyZplS*fn|RPsUd6=TSwOuuEo?=s#xp8Am+#(t9{2ElbJ%>_RD}T0fL(Zyn3)^a_VwH` zAQs7{(uQuH=&N1CH>~CKwxuFS1>O^Ga^2i8d`q<$;OV8lwUMi=AH|X?^;61;Y(`&G z?sr`811p*(p=3psUN@yXYgpBCozF>;xF8YC71Fxwkz)+_OI&a0WmC2ai~1<>>hVn#-)||opYYjxGIExdOY-c}J1gx;e_WL99e*h~4IO!uxEE1@3JoC)e4iR~ z$MG8lIxPI6BSW7>T%;q62!`nXDh*L~7apztVD1