From ab3b1b2ccc9ee49b3a9184b92ac3cfdd9b741cd9 Mon Sep 17 00:00:00 2001 From: James Teh Date: Tue, 30 Nov 2021 02:07:36 +0000 Subject: [PATCH] Bug 1730862 part 1: Make HyperTextAccessibleBase::TransformOffset easier to use. r=eeejay 1. When calculating a start offset, we usually want 0 if the offset can't be transformed, thus clipping to the start of this Accessible. Previously, CharacterCount was always returned even when calculating a start offset. Now, we return 0 for a start offset, CharacterCount for an end offset. 2. Sometimes, this fallback value is insufficient and we need to know explicitly whether it failed. As well as the offset, we now return a boolean indicating this. This will be used in a subsequent patch. 3. Even though TransformOffset only deals with positive numbers, callers usually deal with signed numbers. Therefore, we now use int32_t insteaed of uint32_t to avoid a lot of casting. Differential Revision: https://phabricator.services.mozilla.com/D132094 --- .../basetypes/HyperTextAccessibleBase.cpp | 52 +++++++------------ .../basetypes/HyperTextAccessibleBase.h | 11 ++-- 2 files changed, 26 insertions(+), 37 deletions(-) diff --git a/accessible/basetypes/HyperTextAccessibleBase.cpp b/accessible/basetypes/HyperTextAccessibleBase.cpp index 3b0ca1b29615..6e1c6e525079 100644 --- a/accessible/basetypes/HyperTextAccessibleBase.cpp +++ b/accessible/basetypes/HyperTextAccessibleBase.cpp @@ -182,17 +182,16 @@ TextLeafPoint HyperTextAccessibleBase::ToTextLeafPoint(int32_t aOffset, return TextLeafPoint(child, offset); } -uint32_t HyperTextAccessibleBase::TransformOffset(Accessible* aDescendant, - uint32_t aOffset, - bool aIsEndOffset) const { +std::pair HyperTextAccessibleBase::TransformOffset( + Accessible* aDescendant, int32_t aOffset, bool aIsEndOffset) const { const Accessible* thisAcc = Acc(); // From the descendant, go up and get the immediate child of this hypertext. - uint32_t offset = aOffset; + int32_t offset = aOffset; Accessible* descendant = aDescendant; while (descendant) { Accessible* parent = descendant->Parent(); if (parent == thisAcc) { - return GetChildOffset(descendant) + offset; + return {true, GetChildOffset(descendant) + offset}; } // This offset no longer applies because the passed-in text object is not @@ -211,9 +210,10 @@ uint32_t HyperTextAccessibleBase::TransformOffset(Accessible* aDescendant, descendant = parent; } - // If the given a11y point cannot be mapped into offset relative this - // hypertext offset then return length as fallback value. - return CharacterCount(); + // The given a11y point cannot be mapped to an offset relative to this + // hypertext accessible. Return the start or the end depending on whether this + // is a start ofset or an end offset, thus clipping to the relevant endpoint. + return {false, aIsEndOffset ? static_cast(CharacterCount()) : 0}; } void HyperTextAccessibleBase::TextAtOffset(int32_t aOffset, @@ -257,21 +257,12 @@ void HyperTextAccessibleBase::TextAtOffset(int32_t aOffset, } TextLeafPoint start = origStart.FindBoundary(aBoundaryType, eDirPrevious, /* aIncludeOrigin */ true); - *aStartOffset = - static_cast(TransformOffset(start.mAcc, start.mOffset, - /* aIsEndOffset */ false)); - if (*aStartOffset == static_cast(CharacterCount()) && - (*aStartOffset > static_cast(adjustedOffset) || - start != origStart)) { - // start is before this HyperTextAccessible. In that case, - // Transformoffset will return CharacterCount(), but we want to - // clip to the start of this HyperTextAccessible, not the end. - *aStartOffset = 0; - } + bool ok; + std::tie(ok, *aStartOffset) = TransformOffset(start.mAcc, start.mOffset, + /* aIsEndOffset */ false); end = end.FindBoundary(aBoundaryType, eDirNext); - *aEndOffset = - static_cast(TransformOffset(end.mAcc, end.mOffset, - /* aIsEndOffset */ true)); + std::tie(ok, *aEndOffset) = TransformOffset(end.mAcc, end.mOffset, + /* aIsEndOffset */ true); TextSubstring(*aStartOffset, *aEndOffset, aText); return; } @@ -355,20 +346,13 @@ already_AddRefed HyperTextAccessibleBase::TextAttributes( RefPtr attributes = origin.GetTextAttributes(aIncludeDefAttrs); TextLeafPoint start = origin.FindTextAttrsStart( eDirPrevious, /* aIncludeOrigin */ true, attributes, aIncludeDefAttrs); - *aStartOffset = - static_cast(TransformOffset(start.mAcc, start.mOffset, - /* aIsEndOffset */ false)); - if (*aStartOffset == static_cast(CharacterCount()) && - (*aStartOffset > static_cast(offset) || start != origin)) { - // start is before this HyperTextAccessible. In that case, - // Transformoffset will return CharacterCount(), but we want to - // clip to the start of this HyperTextAccessible, not the end. - *aStartOffset = 0; - } + bool ok; + std::tie(ok, *aStartOffset) = TransformOffset(start.mAcc, start.mOffset, + /* aIsEndOffset */ false); TextLeafPoint end = origin.FindTextAttrsStart( eDirNext, /* aIncludeOrigin */ false, attributes, aIncludeDefAttrs); - *aEndOffset = static_cast(TransformOffset(end.mAcc, end.mOffset, - /* aIsEndOffset */ true)); + std::tie(ok, *aEndOffset) = TransformOffset(end.mAcc, end.mOffset, + /* aIsEndOffset */ true); return attributes.forget(); } diff --git a/accessible/basetypes/HyperTextAccessibleBase.h b/accessible/basetypes/HyperTextAccessibleBase.h index b9a0a1152fbb..8e7bd9230ef6 100644 --- a/accessible/basetypes/HyperTextAccessibleBase.h +++ b/accessible/basetypes/HyperTextAccessibleBase.h @@ -148,10 +148,15 @@ class HyperTextAccessibleBase { private: /** - * Transform the given a11y point into the offset relative this hypertext. + * Transform the given a11y point into an offset relative to this hypertext. + * Returns {success, offset}, where success is true if successful. + * If unsuccessful, the returned offset will be CharacterCount() if + * aIsEndOffset is true, 0 otherwise. This means most callers can ignore the + * success return value. */ - uint32_t TransformOffset(Accessible* aDescendant, uint32_t aOffset, - bool aIsEndOffset) const; + std::pair TransformOffset(Accessible* aDescendant, + int32_t aOffset, + bool aIsEndOffset) const; }; } // namespace mozilla::a11y