From 3934c7d529f39a811f618affe30431e5578ba915 Mon Sep 17 00:00:00 2001 From: James Teh Date: Fri, 11 Feb 2022 02:25:21 +0000 Subject: [PATCH] Bug 1730085 part 4: Implement support for BOUNDARY_WORD_END in TextLeafPoint and HyperTextAccessibleBase. r=eeejay BOUNDARY_WORD_END is implemented using BOUNDARY_WORD_START and adjusting for spaces, which are word end boundaries. This is arguably less efficient than it could be, since we will walk over space and then reverse course to compensate. However, the alternative would mean keeping two slightly different versions of the word boundary check code in sync, plus compensating for the fact that a word often ends before a line start while still supporting words split by line wrapping. I felt the lower complexity here outweighed the potential slight loss in efficiency. We can always revisit this if this turns out to be a real problem. Differential Revision: https://phabricator.services.mozilla.com/D138105 --- accessible/base/TextLeafRange.cpp | 65 +++++++++++++++++++ accessible/base/TextLeafRange.h | 3 + .../basetypes/HyperTextAccessibleBase.cpp | 31 ++++++++- .../basetypes/HyperTextAccessibleBase.h | 3 +- accessible/generic/HyperTextAccessible.cpp | 3 + .../browser/e10s/browser_caching_text.js | 34 ++++++++++ 6 files changed, 135 insertions(+), 4 deletions(-) diff --git a/accessible/base/TextLeafRange.cpp b/accessible/base/TextLeafRange.cpp index b92291d49eaa..558c4e8342cb 100644 --- a/accessible/base/TextLeafRange.cpp +++ b/accessible/base/TextLeafRange.cpp @@ -752,6 +752,9 @@ TextLeafPoint TextLeafPoint::FindBoundary(AccessibleTextBoundary aBoundaryType, if (aBoundaryType == nsIAccessibleText::BOUNDARY_LINE_END) { return FindLineEnd(aDirection, aIncludeOrigin); } + if (aBoundaryType == nsIAccessibleText::BOUNDARY_WORD_END) { + return FindWordEnd(aDirection, aIncludeOrigin); + } if (aBoundaryType == nsIAccessibleText::BOUNDARY_LINE_START && aIncludeOrigin && aDirection == eDirPrevious && IsEmptyLastLine()) { // If we're at an empty line at the end of an Accessible, we don't want to @@ -867,6 +870,68 @@ TextLeafPoint TextLeafPoint::FindLineEnd(nsDirection aDirection, return lineStart; } +bool TextLeafPoint::IsSpace() const { + return GetWordBreakClass(GetChar()) == eWbcSpace; +} + +TextLeafPoint TextLeafPoint::FindWordEnd(nsDirection aDirection, + bool aIncludeOrigin) const { + char16_t origChar = GetChar(); + const bool origIsSpace = GetWordBreakClass(origChar) == eWbcSpace; + bool prevIsSpace = false; + if (aDirection == eDirPrevious || (aIncludeOrigin && origIsSpace) || + !origChar) { + TextLeafPoint prev = + FindBoundary(nsIAccessibleText::BOUNDARY_CHAR, eDirPrevious, false); + if (aDirection == eDirPrevious && prev == *this) { + return *this; // Can't go any further. + } + prevIsSpace = prev.IsSpace(); + if (aIncludeOrigin && origIsSpace && !prevIsSpace) { + // The origin is space, but the previous character is not. This means + // we're at the end of a word. + return *this; + } + } + TextLeafPoint boundary = *this; + if (aDirection == eDirPrevious && !prevIsSpace) { + // If there isn't space immediately before us, first find the start of the + // previous word. + boundary = FindBoundary(nsIAccessibleText::BOUNDARY_WORD_START, + eDirPrevious, aIncludeOrigin); + } else if (aDirection == eDirNext && + (origIsSpace || (!origChar && prevIsSpace))) { + // We're within the space at the end of the word. Skip over the space. We + // can do that by searching for the next word start. + boundary = + FindBoundary(nsIAccessibleText::BOUNDARY_WORD_START, eDirNext, false); + if (boundary.IsSpace()) { + // The next word starts with a space. This can happen if there is a space + // after or at the start of a block element. + return boundary; + } + } + if (aDirection == eDirNext) { + boundary = boundary.FindBoundary(nsIAccessibleText::BOUNDARY_WORD_START, + eDirNext, aIncludeOrigin); + } + // At this point, boundary is either the start of a word or at a space. A + // word ends at the beginning of consecutive space. Therefore, skip back to + // the start of any space before us. + TextLeafPoint prev = boundary; + for (;;) { + prev = prev.FindBoundary(nsIAccessibleText::BOUNDARY_CHAR, eDirPrevious); + if (prev == boundary) { + break; // Can't go any further. + } + if (!prev.IsSpace()) { + break; + } + boundary = prev; + } + return boundary; +} + already_AddRefed TextLeafPoint::GetTextAttributesLocalAcc( bool aIncludeDefaults) const { LocalAccessible* acc = mAcc->AsLocal(); diff --git a/accessible/base/TextLeafRange.h b/accessible/base/TextLeafRange.h index 2044e1389324..4244130afad8 100644 --- a/accessible/base/TextLeafRange.h +++ b/accessible/base/TextLeafRange.h @@ -148,6 +148,8 @@ class TextLeafPoint final { bool IsLineFeedChar() const { return GetChar() == '\n'; } + bool IsSpace() const; + private: bool IsEmptyLastLine() const; @@ -164,6 +166,7 @@ class TextLeafPoint final { bool aIncludeOrigin) const; TextLeafPoint FindLineEnd(nsDirection aDirection, bool aIncludeOrigin) const; + TextLeafPoint FindWordEnd(nsDirection aDirection, bool aIncludeOrigin) const; }; /** diff --git a/accessible/basetypes/HyperTextAccessibleBase.cpp b/accessible/basetypes/HyperTextAccessibleBase.cpp index 173a990c7743..1e8481adf3b5 100644 --- a/accessible/basetypes/HyperTextAccessibleBase.cpp +++ b/accessible/basetypes/HyperTextAccessibleBase.cpp @@ -217,7 +217,8 @@ std::pair HyperTextAccessibleBase::TransformOffset( } void HyperTextAccessibleBase::AdjustOriginIfEndBoundary( - TextLeafPoint& aOrigin, AccessibleTextBoundary aBoundaryType) const { + TextLeafPoint& aOrigin, AccessibleTextBoundary aBoundaryType, + bool aAtOffset) const { if (aBoundaryType != nsIAccessibleText::BOUNDARY_LINE_END && aBoundaryType != nsIAccessibleText::BOUNDARY_WORD_END) { return; @@ -231,6 +232,26 @@ void HyperTextAccessibleBase::AdjustOriginIfEndBoundary( } aOrigin = actualOrig.FindBoundary(nsIAccessibleText::BOUNDARY_CHAR, eDirPrevious); + } else { // BOUNDARY_WORD_END + if (aAtOffset) { + // For TextAtOffset with BOUNDARY_WORD_END, we follow WebKitGtk here and + // return the word which ends after the origin if the origin is a word end + // boundary. Also, if the caret is at the end of a line, our tests expect + // the word after the caret, not the word before. The reason for that + // is a mystery lost to history. We can do that by explicitly using the + // actualized caret without adjusting for end of line. + aOrigin = actualOrig; + return; + } + if (!actualOrig.IsSpace()) { + return; + } + TextLeafPoint prevChar = + actualOrig.FindBoundary(nsIAccessibleText::BOUNDARY_CHAR, eDirPrevious); + if (prevChar != actualOrig && !prevChar.IsSpace()) { + // aOrigin is a word end boundary. + aOrigin = prevChar; + } } } @@ -255,6 +276,7 @@ void HyperTextAccessibleBase::TextBeforeOffset( } break; case nsIAccessibleText::BOUNDARY_WORD_START: + case nsIAccessibleText::BOUNDARY_WORD_END: case nsIAccessibleText::BOUNDARY_LINE_START: case nsIAccessibleText::BOUNDARY_LINE_END: TextLeafPoint orig; @@ -312,12 +334,13 @@ void HyperTextAccessibleBase::TextAtOffset(int32_t aOffset, CharAt(adjustedOffset, aText, aStartOffset, aEndOffset); break; case nsIAccessibleText::BOUNDARY_WORD_START: + case nsIAccessibleText::BOUNDARY_WORD_END: case nsIAccessibleText::BOUNDARY_LINE_START: case nsIAccessibleText::BOUNDARY_LINE_END: TextLeafPoint start, end; if (aOffset == nsIAccessibleText::TEXT_OFFSET_CARET) { start = TextLeafPoint::GetCaret(Acc()); - AdjustOriginIfEndBoundary(start, aBoundaryType); + AdjustOriginIfEndBoundary(start, aBoundaryType, /* aAtOffset */ true); end = start; } else { start = ToTextLeafPoint(static_cast(adjustedOffset)); @@ -333,7 +356,8 @@ void HyperTextAccessibleBase::TextAtOffset(int32_t aOffset, end = ToTextLeafPoint(static_cast(adjustedOffset), /* aDescendToEnd */ true); } else { - AdjustOriginIfEndBoundary(start, aBoundaryType); + AdjustOriginIfEndBoundary(start, aBoundaryType, + /* aAtOffset */ true); end = start; } } @@ -375,6 +399,7 @@ void HyperTextAccessibleBase::TextAfterOffset( break; } case nsIAccessibleText::BOUNDARY_WORD_START: + case nsIAccessibleText::BOUNDARY_WORD_END: case nsIAccessibleText::BOUNDARY_LINE_START: case nsIAccessibleText::BOUNDARY_LINE_END: TextLeafPoint orig; diff --git a/accessible/basetypes/HyperTextAccessibleBase.h b/accessible/basetypes/HyperTextAccessibleBase.h index 1548d4160da5..808909489d1d 100644 --- a/accessible/basetypes/HyperTextAccessibleBase.h +++ b/accessible/basetypes/HyperTextAccessibleBase.h @@ -187,7 +187,8 @@ class HyperTextAccessibleBase { * accordingly. */ void AdjustOriginIfEndBoundary(TextLeafPoint& aOrigin, - AccessibleTextBoundary aBoundaryType) const; + AccessibleTextBoundary aBoundaryType, + bool aAtOffset = false) const; }; } // namespace mozilla::a11y diff --git a/accessible/generic/HyperTextAccessible.cpp b/accessible/generic/HyperTextAccessible.cpp index f0c99a29a19c..296d1fe4671c 100644 --- a/accessible/generic/HyperTextAccessible.cpp +++ b/accessible/generic/HyperTextAccessible.cpp @@ -922,6 +922,7 @@ void HyperTextAccessible::TextBeforeOffset(int32_t aOffset, nsAString& aText) { if (StaticPrefs::accessibility_cache_enabled_AtStartup() && (aBoundaryType == nsIAccessibleText::BOUNDARY_WORD_START || + aBoundaryType == nsIAccessibleText::BOUNDARY_WORD_END || aBoundaryType == nsIAccessibleText::BOUNDARY_LINE_START || aBoundaryType == nsIAccessibleText::BOUNDARY_LINE_END)) { // This isn't strictly related to caching, but this new text implementation @@ -1012,6 +1013,7 @@ void HyperTextAccessible::TextAtOffset(int32_t aOffset, int32_t* aEndOffset, nsAString& aText) { if (StaticPrefs::accessibility_cache_enabled_AtStartup() && (aBoundaryType == nsIAccessibleText::BOUNDARY_WORD_START || + aBoundaryType == nsIAccessibleText::BOUNDARY_WORD_END || aBoundaryType == nsIAccessibleText::BOUNDARY_LINE_START || aBoundaryType == nsIAccessibleText::BOUNDARY_LINE_END)) { // This isn't strictly related to caching, but this new text implementation @@ -1110,6 +1112,7 @@ void HyperTextAccessible::TextAfterOffset(int32_t aOffset, nsAString& aText) { if (StaticPrefs::accessibility_cache_enabled_AtStartup() && (aBoundaryType == nsIAccessibleText::BOUNDARY_WORD_START || + aBoundaryType == nsIAccessibleText::BOUNDARY_WORD_END || aBoundaryType == nsIAccessibleText::BOUNDARY_LINE_START || aBoundaryType == nsIAccessibleText::BOUNDARY_LINE_END)) { // This isn't strictly related to caching, but this new text implementation diff --git a/accessible/tests/browser/e10s/browser_caching_text.js b/accessible/tests/browser/e10s/browser_caching_text.js index 97807809914e..a0caaeeba64c 100644 --- a/accessible/tests/browser/e10s/browser_caching_text.js +++ b/accessible/tests/browser/e10s/browser_caching_text.js @@ -94,6 +94,40 @@ ef gh [6, 8, "gh", 9, 11], [9, 11, "", 11, 11], ]); + if (isWinNoCache) { + todo( + false, + "Cache disabled, so RemoteAccessible doesn't support BOUNDARY_WORD_END on Windows" + ); + } else { + testTextAtOffset(acc, BOUNDARY_WORD_END, [ + [0, 1, "ab", 0, 2], + [2, 4, " cd", 2, 5], + [5, 7, "\nef", 5, 8], + [8, 11, " gh", 8, 11], + ]); + testTextBeforeOffset(acc, BOUNDARY_WORD_END, [ + [0, 2, "", 0, 0], + [3, 5, "ab", 0, 2], + // See below for offset 6. + [7, 8, " cd", 2, 5], + [9, 11, "\nef", 5, 8], + ]); + if (id == "br" && !isCacheEnabled) { + todo( + false, + "Cache disabled, so TextBeforeOffset BOUNDARY_WORD_END returns incorrect result after br" + ); + } else { + testTextBeforeOffset(acc, BOUNDARY_WORD_END, [[6, 6, " cd", 2, 5]]); + } + testTextAfterOffset(acc, BOUNDARY_WORD_END, [ + [0, 2, " cd", 2, 5], + [3, 5, "\nef", 5, 8], + [6, 8, " gh", 8, 11], + [9, 11, "", 11, 11], + ]); + } } const linksStartEnd = findAccessibleChildByID(docAcc, "linksStartEnd"); testTextAtOffset(linksStartEnd, BOUNDARY_LINE_START, [