Skip to content

Commit

Permalink
Handle nodeless break opportunities in AXPosition::FromPosition
Browse files Browse the repository at this point in the history
The accessible text exposed to assistive technologies comes from the
InlineTextBox content; characters representing break opportunities are
excluded.

If the break opportunity has an associated node, such as <wbr>,
AXPosition is able to connect that character's content offsets
with the "ignored" accessibility object, and ensure the text offsets
exposed to ATs correspond to the accessible text. It needs to do the
same for break opportunities which lack an associated node, such as
those inserted at the end of preserved leading spaces.

Create AXPosition::AdjustContentOffsetForNonContiguousMappings which
identifies nodeless break opportunities and adjusts the accessible
content offset accordingly.

AX-Relnotes: Screen readers should no longer present the wrong character
during caret navigation if "white-space: pre-wrap" is applied to an
element with leading spaces.

Bug: 1442835
Change-Id: I914d051b942dcd50de55674816e32cfbf578e5a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4543184
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Joanmarie Diggs <jdiggs@igalia.com>
Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1150216}
  • Loading branch information
joanmarie authored and Chromium LUCI CQ committed May 29, 2023
1 parent ce71723 commit 62bb11d
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 6 deletions.
43 changes: 37 additions & 6 deletions third_party/blink/renderer/modules/accessibility/ax_position.cc
Original file line number Diff line number Diff line change
Expand Up @@ -265,12 +265,15 @@ const AXPosition AXPosition::FromPosition(
// subtract the text offset of our |container| from the beginning of the
// same formatting context.
int container_offset = container->TextOffsetInFormattingContext(0);
int text_offset =
static_cast<int>(
container_offset_mapping
->GetTextContentOffset(parent_anchored_position)
.value_or(static_cast<unsigned int>(container_offset))) -
container_offset;
absl::optional<unsigned> content_offset =
container_offset_mapping->GetTextContentOffset(
parent_anchored_position);
int text_offset = 0;
if (content_offset.has_value()) {
text_offset = ax_position.AdjustContentOffsetForNonContiguousMappings(
container_offset_mapping, content_offset.value()) -
container_offset;
}
DCHECK_GE(text_offset, 0);
ax_position.text_offset_or_child_index_ = text_offset;
ax_position.affinity_ = affinity;
Expand Down Expand Up @@ -1004,6 +1007,34 @@ String AXPosition::ToString() const {
return builder.ToString();
}

int AXPosition::AdjustContentOffsetForNonContiguousMappings(
const NGOffsetMapping* mapping,
int content_offset) const {
if (!mapping) {
return content_offset;
}

String text = mapping->GetText();
int count = 0;
unsigned previous_content_end = 0;
for (auto unit : mapping->GetUnits()) {
if (unit.TextContentStart() > static_cast<unsigned>(content_offset)) {
break;
}

// There should not be multiple contiguous break opportunities for which
// there is no associated mapping unit (e.g. the `wbr` element has a unit).
if (unit.TextContentStart() != previous_content_end &&
text[previous_content_end] == kZeroWidthSpaceCharacter) {
count++;
}

previous_content_end = unit.TextContentEnd();
}

return content_offset - count;
}

// static
const AXObject* AXPosition::FindNeighboringUnignoredObject(
const Document& document,
Expand Down
30 changes: 30 additions & 0 deletions third_party/blink/renderer/modules/accessibility/ax_position.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "base/logging.h"
#include "third_party/blink/renderer/core/editing/forward.h"
#include "third_party/blink/renderer/core/editing/text_affinity.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/platform/heap/persistent.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
Expand Down Expand Up @@ -170,6 +171,35 @@ class MODULES_EXPORT AXPosition final {
const ContainerNode* container_node,
const AXPositionAdjustmentBehavior adjustment_behavior);

// Adjusts the text content offset for non-contiguous mapping units.
// Normally mapping units are contiguous, with each unit's content start
// offset being the same as the previous unit's content end offset. One
// exception to this is the insertion of a break opportunity that does not
// have a corresponding node. Example:
//
// <div contenteditable="true" style="white-space: pre-wrap;"> Bar</div>
//
// `NGOffsetMapping::GetText` returns a string with seven characters:
// * three spaces, mapping unit's content offsets 0-3
// * one `kZeroWidthSpaceCharacter`, no corresponding mapping unit
// * "Bar", mapping unit's content offsets 4-7
// Thus when the caret moves to the "B" in "Bar", the text content offset we
// get from `NGOffsetMapping` is 4. When an AT asks us for the character at
// offset 4 in order to present the new location, we will return "a" if we
// don't adjust the offset for the `kZeroWidthSpaceCharacter`. This mismatch
// is due to the fact that the text we expose to ATs consists of six
// characters taken from:
// * the `InlineTextBox` with three spaces
// * the `InlineTextBox` with "Bar"
//
// Note that `<wbr>`, whose text is also `kZeroWidthSpaceCharacter`, does
// have a mapping unit and corresponding node. This makes it possible for
// us to associate its content offsets with its node, which is an ignored
// object in the accessibility tree.
int AdjustContentOffsetForNonContiguousMappings(
const NGOffsetMapping* mapping,
int content_offset) const;

// The |AXObject| in which the position is present.
// Only valid during a single document lifecycle hence no need to maintain a
// strong reference to it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,37 @@ TEST_F(AccessibilityTest, AXPositionFromDOMPositionWithWhiteSpace) {
EXPECT_EQ(nullptr, ax_position_before_white_space.ChildAfterTreePosition());
}

TEST_F(AccessibilityTest, AXPositionsWithPreservedLeadingWhitespace) {
SetBodyInnerHTML(R"HTML(
<div id="div" style="white-space: pre-wrap;"> Bar</div>
)HTML");

const Node* text = GetElementById("div")->firstChild();
ASSERT_NE(nullptr, text);
EXPECT_TRUE(text->IsTextNode());
EXPECT_EQ(6U, text->textContent().length());

const Position position_at_start(*text, 0);
const auto ax_position_at_start = AXPosition::FromPosition(position_at_start);
EXPECT_TRUE(ax_position_at_start.IsTextPosition());
EXPECT_EQ(0, ax_position_at_start.TextOffset());

// If we didn't adjust for the break opportunity, the accessible text offset
// would be 4 instead of 3.
const Position position_after_white_space(*text, 3);
const auto ax_position_after_white_space =
AXPosition::FromPosition(position_after_white_space);
EXPECT_TRUE(ax_position_after_white_space.IsTextPosition());
EXPECT_EQ(3, ax_position_after_white_space.TextOffset());

// If we didn't adjust for the break opportunity, the accessible text offset
// would be 7 instead of 6.
const Position position_at_end(*text, 6);
const auto ax_position_at_end = AXPosition::FromPosition(position_at_end);
EXPECT_TRUE(ax_position_at_end.IsTextPosition());
EXPECT_EQ(6, ax_position_at_end.TextOffset());
}

//
// Test affinity.
// We need to distinguish between the caret at the end of one line and the
Expand Down

0 comments on commit 62bb11d

Please sign in to comment.