From fc931e5595fb63bf1b2f08460046b18f08e20a85 Mon Sep 17 00:00:00 2001 From: Frederik Gladhorn Date: Wed, 25 Jun 2014 15:27:56 +0200 Subject: [PATCH] Accessibility: Fix boundaries for text functions in QLineEdit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the functions work consistently. For example asking for the line at the cursor position when the cursor was at the end returned an empty line before. Task-number: QTBUG-38500 Change-Id: I60fc78c7be129a59c83efcfce6d8fdd16f2c3f65 Reviewed-by: Jan Arve Sæther --- src/gui/accessible/qaccessible.cpp | 57 ++++++------ src/widgets/accessible/simplewidgets.cpp | 12 +++ .../qaccessibility/tst_qaccessibility.cpp | 87 +++++++++++++++++++ 3 files changed, 125 insertions(+), 31 deletions(-) diff --git a/src/gui/accessible/qaccessible.cpp b/src/gui/accessible/qaccessible.cpp index 7584e23f2e..f41afd52ef 100644 --- a/src/gui/accessible/qaccessible.cpp +++ b/src/gui/accessible/qaccessible.cpp @@ -1917,14 +1917,9 @@ QString QAccessibleTextInterface::textBeforeOffset(int offset, QAccessible::Text { const QString txt = text(0, characterCount()); - if (txt.isEmpty() || offset < 0 || offset > txt.length()) { - *startOffset = *endOffset = -1; + *startOffset = *endOffset = -1; + if (txt.isEmpty() || offset <= 0 || offset > txt.length()) return QString(); - } - if (offset == 0) { - *startOffset = *endOffset = offset; - return QString(); - } QTextBoundaryFinder::BoundaryType type; switch (boundaryType) { @@ -1938,10 +1933,8 @@ QString QAccessibleTextInterface::textBeforeOffset(int offset, QAccessible::Text type = QTextBoundaryFinder::Sentence; break; default: - // in any other case return the whole line - *startOffset = 0; - *endOffset = txt.length(); - return txt; + // return empty, this function currently only supports single lines, so there can be no line before + return QString(); } // keep behavior in sync with QTextCursor::movePosition()! @@ -1977,14 +1970,9 @@ QString QAccessibleTextInterface::textAfterOffset(int offset, QAccessible::TextB { const QString txt = text(0, characterCount()); - if (txt.isEmpty() || offset < 0 || offset > txt.length()) { - *startOffset = *endOffset = -1; + *startOffset = *endOffset = -1; + if (txt.isEmpty() || offset < 0 || offset >= txt.length()) return QString(); - } - if (offset == txt.length()) { - *startOffset = *endOffset = offset; - return QString(); - } QTextBoundaryFinder::BoundaryType type; switch (boundaryType) { @@ -1998,10 +1986,8 @@ QString QAccessibleTextInterface::textAfterOffset(int offset, QAccessible::TextB type = QTextBoundaryFinder::Sentence; break; default: - // in any other case return the whole line - *startOffset = 0; - *endOffset = txt.length(); - return txt; + // return empty, this function currently only supports single lines, so there can be no line after + return QString(); } // keep behavior in sync with QTextCursor::movePosition()! @@ -2009,20 +1995,31 @@ QString QAccessibleTextInterface::textAfterOffset(int offset, QAccessible::TextB QTextBoundaryFinder boundary(type, txt); boundary.setPosition(offset); - while (boundary.toNextBoundary() < txt.length()) { + while (true) { + int toNext = boundary.toNextBoundary(); if ((boundary.boundaryReasons() & (QTextBoundaryFinder::StartOfItem | QTextBoundaryFinder::EndOfItem))) break; + if (toNext < 0 || toNext >= txt.length()) + break; // not found, the boundary might not exist } Q_ASSERT(boundary.position() <= txt.length()); *startOffset = boundary.position(); - while (boundary.toNextBoundary() < txt.length()) { + while (true) { + int toNext = boundary.toNextBoundary(); if ((boundary.boundaryReasons() & (QTextBoundaryFinder::StartOfItem | QTextBoundaryFinder::EndOfItem))) break; + if (toNext < 0 || toNext >= txt.length()) + break; // not found, the boundary might not exist } Q_ASSERT(boundary.position() <= txt.length()); *endOffset = boundary.position(); + if ((*startOffset == -1) || (*endOffset == -1) || (*startOffset == *endOffset)) { + *endOffset = -1; + *startOffset = -1; + } + return txt.mid(*startOffset, *endOffset - *startOffset); } @@ -2037,14 +2034,12 @@ QString QAccessibleTextInterface::textAtOffset(int offset, QAccessible::TextBoun { const QString txt = text(0, characterCount()); - if (txt.isEmpty() || offset < 0 || offset > txt.length()) { - *startOffset = *endOffset = -1; + *startOffset = *endOffset = -1; + if (txt.isEmpty() || offset < 0 || offset > txt.length()) return QString(); - } - if (offset == txt.length()) { - *startOffset = *endOffset = offset; + + if (offset == txt.length() && boundaryType == QAccessible::CharBoundary) return QString(); - } QTextBoundaryFinder::BoundaryType type; switch (boundaryType) { @@ -2058,7 +2053,7 @@ QString QAccessibleTextInterface::textAtOffset(int offset, QAccessible::TextBoun type = QTextBoundaryFinder::Sentence; break; default: - // in any other case return the whole line + // return the whole line *startOffset = 0; *endOffset = txt.length(); return txt; diff --git a/src/widgets/accessible/simplewidgets.cpp b/src/widgets/accessible/simplewidgets.cpp index 88d3981391..6fd249f372 100644 --- a/src/widgets/accessible/simplewidgets.cpp +++ b/src/widgets/accessible/simplewidgets.cpp @@ -752,6 +752,10 @@ QString QAccessibleLineEdit::textBeforeOffset(int offset, QAccessible::TextBound *startOffset = *endOffset = -1; return QString(); } + if (offset == -1) + offset = lineEdit()->text().length(); + if (offset == -2) + offset = cursorPosition(); return QAccessibleTextInterface::textBeforeOffset(offset, boundaryType, startOffset, endOffset); } @@ -762,6 +766,10 @@ QString QAccessibleLineEdit::textAfterOffset(int offset, QAccessible::TextBounda *startOffset = *endOffset = -1; return QString(); } + if (offset == -1) + offset = lineEdit()->text().length(); + if (offset == -2) + offset = cursorPosition(); return QAccessibleTextInterface::textAfterOffset(offset, boundaryType, startOffset, endOffset); } @@ -772,6 +780,10 @@ QString QAccessibleLineEdit::textAtOffset(int offset, QAccessible::TextBoundaryT *startOffset = *endOffset = -1; return QString(); } + if (offset == -1) + offset = lineEdit()->text().length(); + if (offset == -2) + offset = cursorPosition(); return QAccessibleTextInterface::textAtOffset(offset, boundaryType, startOffset, endOffset); } diff --git a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp index c9df497b07..88f2120e62 100644 --- a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp +++ b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp @@ -264,6 +264,8 @@ private slots: void mdiAreaTest(); void mdiSubWindowTest(); void lineEditTest(); + void lineEditTextFunctions_data(); + void lineEditTextFunctions(); void groupBoxTest(); void dialogButtonBoxTest(); void dialTest(); @@ -2293,6 +2295,91 @@ void tst_QAccessibility::lineEditTest() QTestAccessibility::clearEvents(); } +void tst_QAccessibility::lineEditTextFunctions_data() +{ + QTest::addColumn("text"); + QTest::addColumn("textFunction"); // before = 0, at = 1, after = 2 + QTest::addColumn("boundaryType"); + QTest::addColumn("cursorPosition"); + QTest::addColumn("offset"); + QTest::addColumn("expectedStart"); + QTest::addColumn("expectedEnd"); + QTest::addColumn("expectedText"); + + // -2 gives cursor position, -1 is length + // invalid positions will return empty strings and either -1 and -1 or both the cursor position, both is fine + QTest::newRow("char before -2") << "hello" << 0 << (int) QAccessible::CharBoundary << 3 << -2 << 2 << 3 << "l"; + QTest::newRow("char at -2") << "hello" << 1 << (int) QAccessible::CharBoundary << 3 << -2 << 3 << 4 << "l"; + QTest::newRow("char after -2") << "hello" << 2 << (int) QAccessible::CharBoundary << 3 << -2 << 4 << 5 << "o"; + QTest::newRow("char before -1") << "hello" << 0 << (int) QAccessible::CharBoundary << 3 << -1 << 4 << 5 << "o"; + QTest::newRow("char at -1") << "hello" << 1 << (int) QAccessible::CharBoundary << 3 << -1 << -1 << -1 << ""; + QTest::newRow("char after -1") << "hello" << 2 << (int) QAccessible::CharBoundary << 3 << -1 << -1 << -1 << ""; + QTest::newRow("char before 0") << "hello" << 0 << (int) QAccessible::CharBoundary << 0 << 0 << -1 << -1 << ""; + QTest::newRow("char at 0") << "hello" << 1 << (int) QAccessible::CharBoundary << 0 << 0 << 0 << 1 << "h"; + QTest::newRow("char after 0") << "hello" << 2 << (int) QAccessible::CharBoundary << 0 << 0 << 1 << 2 << "e"; + QTest::newRow("char before 1") << "hello" << 0 << (int) QAccessible::CharBoundary << 3 << 1 << 0 << 1 << "h"; + QTest::newRow("char at 1") << "hello" << 1 << (int) QAccessible::CharBoundary << 3 << 1 << 1 << 2 << "e"; + QTest::newRow("char after 1") << "hello" << 2 << (int) QAccessible::CharBoundary << 3 << 1 << 2 << 3 << "l"; + QTest::newRow("char before 4") << "hello" << 0 << (int) QAccessible::CharBoundary << 3 << 4 << 3 << 4 << "l"; + QTest::newRow("char at 4") << "hello" << 1 << (int) QAccessible::CharBoundary << 3 << 4 << 4 << 5 << "o"; + QTest::newRow("char after 4") << "hello" << 2 << (int) QAccessible::CharBoundary << 3 << 4 << -1 << -1 << ""; + QTest::newRow("char before 5") << "hello" << 0 << (int) QAccessible::CharBoundary << 3 << 5 << 4 << 5 << "o"; + QTest::newRow("char at 5") << "hello" << 1 << (int) QAccessible::CharBoundary << 3 << 5 << -1 << -1 << ""; + QTest::newRow("char after 5") << "hello" << 2 << (int) QAccessible::CharBoundary << 3 << 5 << -1 << -1 << ""; + QTest::newRow("char before 6") << "hello" << 0 << (int) QAccessible::CharBoundary << 3 << 6 << -1 << -1 << ""; + QTest::newRow("char at 6") << "hello" << 1 << (int) QAccessible::CharBoundary << 3 << 6 << -1 << -1 << ""; + QTest::newRow("char after 6") << "hello" << 2 << (int) QAccessible::CharBoundary << 3 << 6 << -1 << -1 << ""; + + for (int i = -2; i < 6; ++i) { + QTest::newRow(QString::fromLatin1("line before %1").arg(i).toLocal8Bit().constData()) << "hello" << 0 << (int) QAccessible::LineBoundary << 3 << i << -1 << -1 << ""; + QTest::newRow(QString::fromLatin1("line at %1").arg(i).toLocal8Bit().constData()) << "hello" << 1 << (int) QAccessible::LineBoundary << 3 << i << 0 << 5 << "hello"; + QTest::newRow(QString::fromLatin1("line after %1").arg(i).toLocal8Bit().constData()) << "hello" << 2 << (int) QAccessible::LineBoundary << 3 << i << -1 << -1 << ""; + } +} + +void tst_QAccessibility::lineEditTextFunctions() +{ + { + QFETCH(QString, text); + QFETCH(int, textFunction); + QFETCH(int, boundaryType); + QFETCH(int, cursorPosition); + QFETCH(int, offset); + QFETCH(int, expectedStart); + QFETCH(int, expectedEnd); + QFETCH(QString, expectedText); + + QLineEdit le; + le.show(); + le.setText(text); + le.setCursorPosition(cursorPosition); + + QAccessibleInterface *iface = QAccessible::queryAccessibleInterface(&le); + QVERIFY(iface); + QAccessibleTextInterface *textIface = iface->textInterface(); + QVERIFY(textIface); + + int start = -33; + int end = -33; + QString result; + switch (textFunction) { + case 0: + result = textIface->textBeforeOffset(offset, (QAccessible::TextBoundaryType) boundaryType, &start, &end); + break; + case 1: + result = textIface->textAtOffset(offset, (QAccessible::TextBoundaryType) boundaryType, &start, &end); + break; + case 2: + result = textIface->textAfterOffset(offset, (QAccessible::TextBoundaryType) boundaryType, &start, &end); + break; + } + QCOMPARE(result, expectedText); + QCOMPARE(start, expectedStart); + QCOMPARE(end, expectedEnd); + } + QTestAccessibility::clearEvents(); +} + void tst_QAccessibility::groupBoxTest() { {