Revert "Handle nodeless break opportunities in AXPosition::FromPosition"
This reverts commit 62bb11dfc61d7b9af789ffa6d3c4feca8dcfca81.
Reason for revert: We are hitting a DCHECK (see bug 1450024).
Original change's description:
> Handle nodeless break opportunities in AXPosition::FromPosition
>
> 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}
Bug: 1442835
Change-Id: Ib83e7802083788c1465243c6d3e5e1f0f2e939b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4577058
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Auto-Submit: Joanmarie Diggs <jdiggs@igalia.com>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1151386}
diff --git a/third_party/blink/renderer/modules/accessibility/ax_position.cc b/third_party/blink/renderer/modules/accessibility/ax_position.cc
index 575494fc..3ef3b3d 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_position.cc
+++ b/third_party/blink/renderer/modules/accessibility/ax_position.cc
@@ -265,15 +265,12 @@
// subtract the text offset of our |container| from the beginning of the
// same formatting context.
int container_offset = container->TextOffsetInFormattingContext(0);
- 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;
- }
+ int text_offset =
+ static_cast<int>(
+ container_offset_mapping
+ ->GetTextContentOffset(parent_anchored_position)
+ .value_or(static_cast<unsigned int>(container_offset))) -
+ container_offset;
DCHECK_GE(text_offset, 0);
ax_position.text_offset_or_child_index_ = text_offset;
ax_position.affinity_ = affinity;
@@ -1007,34 +1004,6 @@
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,
diff --git a/third_party/blink/renderer/modules/accessibility/ax_position.h b/third_party/blink/renderer/modules/accessibility/ax_position.h
index a317564..a555ed8 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_position.h
+++ b/third_party/blink/renderer/modules/accessibility/ax_position.h
@@ -13,7 +13,6 @@
#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"
@@ -171,35 +170,6 @@
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.
diff --git a/third_party/blink/renderer/modules/accessibility/ax_position_test.cc b/third_party/blink/renderer/modules/accessibility/ax_position_test.cc
index 0a92158..4de0a5f 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_position_test.cc
+++ b/third_party/blink/renderer/modules/accessibility/ax_position_test.cc
@@ -640,37 +640,6 @@
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
