From a58ec950f06c8de366b4b693491eaaafae116041 Mon Sep 17 00:00:00 2001 From: Frederik Gladhorn Date: Mon, 23 Jun 2014 17:57:34 +0200 Subject: [PATCH] Accessibility: improve text attribute ranges Improve consistency and use QTextDocument functions to find ranges instead of coming up with our own scheme. This is important since QCursor's char format depends on block positions. Change-Id: I94eb137882dc6b5f7b01fa7693b4a536cc48d02a Reviewed-by: Pierre Rossi --- src/widgets/accessible/qaccessiblewidgets.cpp | 67 ++++---- src/widgets/widgets/qwidgettextcontrol.cpp | 4 +- .../qaccessibility/tst_qaccessibility.cpp | 145 ++++++++++++------ 3 files changed, 137 insertions(+), 79 deletions(-) diff --git a/src/widgets/accessible/qaccessiblewidgets.cpp b/src/widgets/accessible/qaccessiblewidgets.cpp index d695bdcbc4..f0a8ecf608 100644 --- a/src/widgets/accessible/qaccessiblewidgets.cpp +++ b/src/widgets/accessible/qaccessiblewidgets.cpp @@ -706,51 +706,48 @@ QString QAccessibleTextWidget::attributes(int offset, int *startOffset, int *end http://linuxfoundation.org/collaborate/workgroups/accessibility/iaccessible2/textattributes */ - if (offset >= characterCount()) { + // IAccessible2 defines -1 as length and -2 as cursor position + if (offset == -2) + offset = cursorPosition(); + + const int charCount = characterCount(); + + // -1 doesn't make much sense here, but it's better to return something + // screen readers may ask for text attributes at the cursor pos which may be equal to length + if (offset == -1 || offset == charCount) + offset = charCount - 1; + + if (offset < 0 || offset > charCount) { *startOffset = -1; *endOffset = -1; return QString(); } - QMap attrs; QTextCursor cursor = textCursor(); - - //cursor.charFormat returns the format of the previous character - cursor.setPosition(offset + 1); - QTextCharFormat charFormat = cursor.charFormat(); - cursor.setPosition(offset); + QTextBlock block = cursor.block(); + + int blockStart = block.position(); + int blockEnd = blockStart + block.length(); + + QTextBlock::iterator iter = block.begin(); + while (!iter.fragment().contains(offset)) + ++iter; + + QTextFragment fragment = iter.fragment(); + int pos = fragment.position(); + + // text block and fragment may overlap, use the smallest common range + *startOffset = qMax(pos, blockStart); + Q_ASSERT(*startOffset <= offset); + *endOffset = qMin(pos + fragment.length(), blockEnd); + Q_ASSERT(*endOffset >= offset); + + QTextCharFormat charFormat = fragment.charFormat(); QTextBlockFormat blockFormat = cursor.blockFormat(); - QTextCharFormat charFormatComp; - QTextBlockFormat blockFormatComp; - - *startOffset = offset; - cursor.setPosition(*startOffset); - while (*startOffset > 0) { - charFormatComp = cursor.charFormat(); - cursor.setPosition(*startOffset - 1); - blockFormatComp = cursor.blockFormat(); - if ((charFormat == charFormatComp) && (blockFormat == blockFormatComp)) - (*startOffset)--; - else - break; - } - - int limit = characterCount() + 1; - *endOffset = offset + 1; - cursor.setPosition(*endOffset); - while (*endOffset < limit) { - blockFormatComp = cursor.blockFormat(); - cursor.setPosition(*endOffset + 1); - charFormatComp = cursor.charFormat(); - if ((charFormat == charFormatComp) && (cursor.blockFormat() == blockFormatComp)) - (*endOffset)++; - else - break; - } - + QMap attrs; QString family = charFormat.fontFamily(); if (!family.isEmpty()) { family = family.replace('\\',QStringLiteral("\\\\")); diff --git a/src/widgets/widgets/qwidgettextcontrol.cpp b/src/widgets/widgets/qwidgettextcontrol.cpp index 54b20e36f5..da348e4edc 100644 --- a/src/widgets/widgets/qwidgettextcontrol.cpp +++ b/src/widgets/widgets/qwidgettextcontrol.cpp @@ -651,7 +651,9 @@ void QWidgetTextControlPrivate::_q_contentsChanged(int from, int charsRemoved, i if (QAccessible::isActive() && q->parent() && q->parent()->isWidgetType()) { QTextCursor tmp(doc); tmp.setPosition(from); - tmp.setPosition(from + charsAdded, QTextCursor::KeepAnchor); + // when setting a new text document the length is off + // QTBUG-32583 - characterCount is off by 1 requires the -1 + tmp.setPosition(qMin(doc->characterCount() - 1, from + charsAdded), QTextCursor::KeepAnchor); QString newText = tmp.selectedText(); // always report the right number of removed chars, but in lack of the real string use spaces diff --git a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp index d6b193a288..23b48212ad 100644 --- a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp +++ b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp @@ -243,6 +243,7 @@ private slots: void statesStructTest(); void navigateHierarchy(); void sliderTest(); + void textAttributes_data(); void textAttributes(); void hideShowTest(); @@ -698,58 +699,118 @@ void tst_QAccessibility::accessibleName() QTestAccessibility::clearEvents(); } -void tst_QAccessibility::textAttributes() +// note: color should probably always be part of the attributes +void tst_QAccessibility::textAttributes_data() { - QTextEdit textEdit; - int startOffset; - int endOffset; - QString attributes; - QString text("" + QTest::addColumn("text"); + QTest::addColumn("offset"); + QTest::addColumn("startOffsetResult"); + QTest::addColumn("endOffsetResult"); + QTest::addColumn("attributeResult"); + + static QStringList defaults = QString("font-style:normal;font-weight:normal;text-align:left;text-position:baseline;text-underline-style:none").split(';'); + static QStringList bold = defaults; + bold[1] = QString::fromLatin1("font-weight:bold"); + + static QStringList italic = defaults; + italic[0] = QString::fromLatin1("font-style:italic"); + + static QStringList boldItalic = defaults; + boldItalic[0] = QString::fromLatin1("font-style:italic"); + boldItalic[1] = QString::fromLatin1("font-weight:bold"); + + static QStringList monospace = defaults; + monospace.append(QLatin1String("font-family:\"monospace\"")); + + static QStringList font8pt = defaults; + font8pt.append(QLatin1String("font-size:8pt")); + + static QStringList color = defaults; + color << QLatin1String("color:rgb(240,241,242)") << QLatin1String("background-color:rgb(20,240,30)"); + + static QStringList rightAlign = defaults; + rightAlign[2] = QStringLiteral("text-align:right"); + + QTest::newRow("defaults 1") << "hello" << 0 << 0 << 5 << defaults; + QTest::newRow("defaults 2") << "hello" << 1 << 0 << 5 << defaults; + QTest::newRow("defaults 3") << "hello" << 4 << 0 << 5 << defaults; + QTest::newRow("defaults 4") << "hello" << 5 << 0 << 5 << defaults; + QTest::newRow("offset -1 length") << "hello" << -1 << 0 << 5 << defaults; + QTest::newRow("offset -2 cursor pos") << "hello" << -2 << 0 << 5 << defaults; + QTest::newRow("offset -3") << "hello" << -3 << -1 << -1 << QStringList(); + QTest::newRow("invalid offset 2") << "hello" << 6 << -1 << -1 << QStringList(); + QTest::newRow("invalid offset 3") << "" << 1 << -1 << -1 << QStringList(); + + QString boldText = QLatin1String("boldtext"); + QTest::newRow("bold 0") << boldText << 0 << 0 << 4 << bold; + QTest::newRow("bold 2") << boldText << 2 << 0 << 4 << bold; + QTest::newRow("bold 3") << boldText << 3 << 0 << 4 << bold; + QTest::newRow("bold 4") << boldText << 4 << 4 << 8 << defaults; + QTest::newRow("bold 6") << boldText << 6 << 4 << 8 << defaults; + + QString longText = QLatin1String("" "Hello, this is an example text." "Multiple fonts are used." "Multiple text sizes are used." - "Let's give some color to Qt." - ""); + "Let's give some color to Qt."); + QTest::newRow("default 5") << longText << 6 << 0 << 7 << defaults; + QTest::newRow("default 6") << longText << 7 << 7 << 11 << bold; + QTest::newRow("bold 7") << longText << 10 << 7 << 11 << bold; + QTest::newRow("bold 8") << longText << 10 << 7 << 11 << bold; + QTest::newRow("bold italic") << longText << 18 << 18 << 25 << boldItalic; + QTest::newRow("monospace") << longText << 34 << 31 << 55 << monospace; + QTest::newRow("8pt") << longText << 65 << 64 << 74 << font8pt; + QTest::newRow("color") << longText << 110 << 109 << 111 << color; + + QString rightAligned = QLatin1String("

right

"); + QTest::newRow("right aligned 1") << rightAligned << 0 << 0 << 5 << rightAlign; + QTest::newRow("right aligned 2") << rightAligned << 1 << 0 << 5 << rightAlign; + QTest::newRow("right aligned 3") << rightAligned << 5 << 0 << 5 << rightAlign; + + // left \n right \n left, make sure bold and alignment borders coincide + QString leftRightLeftAligned = QLatin1String("

left

right

left

"); + QTest::newRow("left right left aligned 1") << leftRightLeftAligned << 1 << 0 << 4 << bold; + QTest::newRow("left right left aligned 3") << leftRightLeftAligned << 3 << 0 << 4 << bold; + QTest::newRow("left right left aligned 4") << leftRightLeftAligned << 4 << 4 << 5 << defaults; + QTest::newRow("left right left aligned 5") << leftRightLeftAligned << 5 << 5 << 10 << rightAlign; + QTest::newRow("left right left aligned 8") << leftRightLeftAligned << 8 << 5 << 10 << rightAlign; + QTest::newRow("left right left aligned 9") << leftRightLeftAligned << 9 << 5 << 10 << rightAlign; + QTest::newRow("left right left aligned 10") << leftRightLeftAligned << 10 << 10 << 11 << rightAlign; + QTest::newRow("left right left aligned 11") << leftRightLeftAligned << 11 << 11 << 15 << bold; + QTest::newRow("left right left aligned 15") << leftRightLeftAligned << 15 << 11 << 15 << bold; +} + +void tst_QAccessibility::textAttributes() +{ + { + QFETCH(QString, text); + QFETCH(int, offset); + QFETCH(int, startOffsetResult); + QFETCH(int, endOffsetResult); + QFETCH(QStringList, attributeResult); + + QTextEdit textEdit; textEdit.setText(text); + if (textEdit.document()->characterCount() > 1) + textEdit.textCursor().setPosition(1); QAccessibleInterface *interface = QAccessible::queryAccessibleInterface(&textEdit); - QAccessibleTextInterface *textInterface=interface->textInterface(); - QVERIFY(textInterface); - QCOMPARE(textInterface->characterCount(), 112); + QCOMPARE(textInterface->characterCount(), textEdit.toPlainText().length()); - attributes = textInterface->attributes(10, &startOffset, &endOffset); - QCOMPARE(startOffset, 7); - QCOMPARE(endOffset, 11); - attributes.prepend(';'); - QVERIFY(attributes.contains(QLatin1String(";font-weight:bold;"))); + int startOffset = -1; + int endOffset = -1; + QString attributes = textInterface->attributes(offset, &startOffset, &endOffset); - attributes = textInterface->attributes(18, &startOffset, &endOffset); - QCOMPARE(startOffset, 18); - QCOMPARE(endOffset, 25); - attributes.prepend(';'); - QVERIFY(attributes.contains(QLatin1String(";font-weight:bold;"))); - QVERIFY(attributes.contains(QLatin1String(";font-style:italic;"))); - - attributes = textInterface->attributes(34, &startOffset, &endOffset); - QCOMPARE(startOffset, 31); - QCOMPARE(endOffset, 55); - attributes.prepend(';'); - QVERIFY(attributes.contains(QLatin1String(";font-family:\"monospace\";"))); - - attributes = textInterface->attributes(65, &startOffset, &endOffset); - QCOMPARE(startOffset, 64); - QCOMPARE(endOffset, 74); - attributes.prepend(';'); - QVERIFY(attributes.contains(QLatin1String(";font-size:8pt;"))); - - attributes = textInterface->attributes(110, &startOffset, &endOffset); - QCOMPARE(startOffset, 109); - QCOMPARE(endOffset, 111); - attributes.prepend(';'); - QVERIFY(attributes.contains(QLatin1String(";background-color:rgb(20,240,30);"))); - QVERIFY(attributes.contains(QLatin1String(";color:rgb(240,241,242);"))); + QCOMPARE(startOffset, startOffsetResult); + QCOMPARE(endOffset, endOffsetResult); + QStringList attrList = attributes.split(QChar(';'), QString::SkipEmptyParts); + attributeResult.sort(); + attrList.sort(); + QCOMPARE(attrList, attributeResult); + } + QTestAccessibility::clearEvents(); } void tst_QAccessibility::hideShowTest() @@ -1763,8 +1824,6 @@ void tst_QAccessibility::textEditTest() QAccessibleTextRemoveEvent remove(&edit, 0, " "); QVERIFY_EVENT(&remove); - // FIXME the new text is not there yet - QEXPECT_FAIL("", "Inserting should always contain the new text", Continue); QAccessibleTextInsertEvent insert(&edit, 0, "Accessibility rocks"); QVERIFY_EVENT(&insert); }