Commit ba464c7b authored by Ethan Jimenez's avatar Ethan Jimenez Committed by Commit Bot

Fix UIA Move(TextUnit_Line) behavior around inline blocks

1. Updating functionality of `ITextRangeProvider::MoveEndpointByUnit` to
   reflect UIA-specific behavior expected by ATs when moving by line and
   crossing the start of an inline block boundary, which should be
   effectively treated as a line break.

   This is mostly implemented in `AXPlatformNodeTextRangeProviderWin`,
   but a new method `AtStartOfInlineBlock` is introduced in `AXPosition`
   to expose inline block boundaries similar to how paragraph boundaries
   are computed by using an `AbortMovePredicate`.

2. Adding browser tests to cover new expectations mentioned above.

Bug: 928948
Change-Id: Ie243a96885edaf3037c75f375ae4fce20fa29e30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1858518
Commit-Queue: Kurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: default avatarKurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: default avatarNektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709093}
parent 898f684b
......@@ -1745,6 +1745,7 @@ IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
"text</span> and text after.",
{L"a ", L"and ", L"text ", L"after."});
// The following test should be enabled when crbug.com/1015100 is fixed.
// AssertMoveByUnitForMarkup(
// TextUnit_Word, "<ul><li>item one</li><li>item two</li></ul>",
// {L"* ", L"item ", L"one\n", L"* ", L"item ", L"two"});
......@@ -1808,6 +1809,32 @@ IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
{L"before ", L"bold text", L"italic text", L" after"});
}
IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
EntireMarkupSuccessiveMoveByLine) {
AssertMoveByUnitForMarkup(TextUnit_Line, "<div style='width:0'>one two</div>",
{L"one ", L"two"});
AssertMoveByUnitForMarkup(TextUnit_Line, "line one<br>line two",
{L"line one", L"line two"});
AssertMoveByUnitForMarkup(TextUnit_Line,
"<div>line one</div><div><div>line two</div></div>",
{L"line one", L"line two"});
AssertMoveByUnitForMarkup(
TextUnit_Line, "<div style='display:inline-block'>a</div>", {L"a"});
AssertMoveByUnitForMarkup(TextUnit_Line,
"a<div style='display:inline-block'>b</div>c",
{L"a", L"b"});
// The following test should be enabled when crbug.com/1015100 is fixed.
// AssertMoveByUnitForMarkup(
// TextUnit_Line,
// "<h1>line one</h1><ul><li>line two</li><li>line three</li></ul>",
// {L"line one\n", L"* line two\n", L"* line three"});
}
IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
BoundingRectangleOfWordBeforeListItemMarker) {
LoadInitialAccessibilityTreeFromHtml(
......
......@@ -663,6 +663,50 @@ class AXPosition {
return next_position->IsNullPosition();
}
bool AtStartOfInlineBlock() const {
AXPositionInstance text_position = AsLeafTextPosition();
switch (text_position->kind_) {
case AXPositionKind::NULL_POSITION:
return false;
case AXPositionKind::TREE_POSITION:
NOTREACHED();
return false;
case AXPositionKind::TEXT_POSITION: {
if (text_position->AtStartOfAnchor()) {
AXPositionInstance previous_position =
text_position->CreatePreviousLeafTreePosition();
// Check that this position is not the start of the first anchor.
if (!previous_position->IsNullPosition()) {
previous_position = text_position->CreatePreviousLeafTreePosition(
base::BindRepeating(&AbortMoveAtStartOfInlineBlock));
// If we get a null position here it means we have crossed an inline
// block's start, thus this position is located at such start.
if (previous_position->IsNullPosition())
return true;
}
}
if (text_position->AtEndOfAnchor()) {
AXPositionInstance next_position =
text_position->CreateNextLeafTreePosition();
// Check that this position is not the end of the last anchor.
if (!next_position->IsNullPosition()) {
next_position = text_position->CreateNextLeafTreePosition(
base::BindRepeating(&AbortMoveAtStartOfInlineBlock));
// If we get a null position here it means we have crossed an inline
// block's start, thus this position is located at such start.
if (next_position->IsNullPosition())
return true;
}
}
return false;
}
}
}
bool AtStartOfDocument() const {
if (IsNullPosition())
return false;
......@@ -2751,6 +2795,37 @@ class AXPosition {
return false;
}
static bool AbortMoveAtStartOfInlineBlock(const AXPosition& move_from,
const AXPosition& move_to,
const AXMoveType move_type,
const AXMoveDirection direction) {
if (move_from.IsNullPosition() || move_to.IsNullPosition())
return true;
// These will only be available if AXMode has kHTML set.
const bool move_from_is_inline_block =
move_from.GetAnchor()->GetStringAttribute(
ax::mojom::StringAttribute::kDisplay) == "inline-block";
const bool move_to_is_inline_block =
move_to.GetAnchor()->GetStringAttribute(
ax::mojom::StringAttribute::kDisplay) == "inline-block";
switch (direction) {
case AXMoveDirection::kNextInTree:
// When moving forward, break if we enter an inline block.
return move_to_is_inline_block &&
(move_type == AXMoveType::kDescendant ||
move_type == AXMoveType::kSibling);
case AXMoveDirection::kPreviousInTree:
// When moving backward, break if we exit an inline block.
return move_from_is_inline_block &&
(move_type == AXMoveType::kAncestor ||
move_type == AXMoveType::kSibling);
}
NOTREACHED();
return false;
}
AXPositionInstance CreateDocumentAncestorPosition() const {
AXPositionInstance iterator = Clone();
while (!iterator->IsNullPosition()) {
......
......@@ -221,10 +221,16 @@ STDMETHODIMP AXPlatformNodeTextRangeProviderWin::ExpandToEnclosingUnit(
}
break;
case TextUnit_Line:
start_ = start_->CreatePreviousLineStartPosition(
AXBoundaryBehavior::StopIfAlreadyAtBoundary);
end_ = start_->CreateNextLineEndPosition(
AXBoundaryBehavior::StopIfAlreadyAtBoundary);
start_ = start_->CreateBoundaryStartPosition(
AXBoundaryBehavior::StopIfAlreadyAtBoundary,
AXTextBoundaryDirection::kBackwards,
base::BindRepeating(&AtStartOfLinePredicate),
base::BindRepeating(&AtEndOfLinePredicate));
end_ = start_->CreateBoundaryEndPosition(
AXBoundaryBehavior::StopIfAlreadyAtBoundary,
AXTextBoundaryDirection::kForwards,
base::BindRepeating(&AtStartOfLinePredicate),
base::BindRepeating(&AtEndOfLinePredicate));
break;
case TextUnit_Paragraph:
start_ = start_->CreatePreviousParagraphStartPosition(
......@@ -867,6 +873,20 @@ STDMETHODIMP AXPlatformNodeTextRangeProviderWin::GetChildren(
return S_OK;
}
// static
bool AXPlatformNodeTextRangeProviderWin::AtStartOfLinePredicate(
const AXPositionInstance& position) {
return !position->IsIgnored() &&
(position->AtStartOfLine() || position->AtStartOfInlineBlock());
}
// static
bool AXPlatformNodeTextRangeProviderWin::AtEndOfLinePredicate(
const AXPositionInstance& position) {
return !position->IsIgnored() &&
(position->AtEndOfLine() || position->AtStartOfInlineBlock());
}
base::string16 AXPlatformNodeTextRangeProviderWin::GetString(int max_count) {
AXNodeRange range(start_->Clone(), end_->Clone());
return range.GetText(AXTextConcatenationBehavior::kAsInnerText, max_count);
......@@ -916,10 +936,42 @@ AXPlatformNodeTextRangeProviderWin::MoveEndpointByLine(
bool is_start_endpoint,
const int count,
int* units_moved) {
return MoveEndpointByUnitHelper(
std::move(endpoint),
is_start_endpoint ? AXTextBoundary::kLineStart : AXTextBoundary::kLineEnd,
count, units_moved);
DCHECK_NE(count, 0);
const bool going_forward = count > 0;
AXPositionInstance current_endpoint = endpoint->Clone();
for (int iteration = 0; iteration < std::abs(count); ++iteration) {
AXPositionInstance next_endpoint;
if (is_start_endpoint) {
next_endpoint = current_endpoint->CreateBoundaryStartPosition(
AXBoundaryBehavior::StopAtLastAnchorBoundary,
going_forward ? AXTextBoundaryDirection::kForwards
: AXTextBoundaryDirection::kBackwards,
base::BindRepeating(&AtStartOfLinePredicate),
base::BindRepeating(&AtEndOfLinePredicate));
} else {
next_endpoint = current_endpoint->CreateBoundaryEndPosition(
AXBoundaryBehavior::StopAtLastAnchorBoundary,
going_forward ? AXTextBoundaryDirection::kForwards
: AXTextBoundaryDirection::kBackwards,
base::BindRepeating(&AtStartOfLinePredicate),
base::BindRepeating(&AtEndOfLinePredicate));
}
DCHECK(!next_endpoint->IsNullPosition());
// Since AXBoundaryBehavior::StopAtLastAnchorBoundary forces the next text
// boundary position to be different than the input position, the only case
// where these are equal is when they're already located at the last anchor
// boundary. In such case, there is no next position to move to.
if (*current_endpoint == *next_endpoint) {
*units_moved = going_forward ? iteration : -iteration;
return current_endpoint;
}
current_endpoint = std::move(next_endpoint);
}
*units_moved = count;
return current_endpoint;
}
AXPlatformNodeTextRangeProviderWin::AXPositionInstance
......
......@@ -89,6 +89,9 @@ class AX_EXPORT __declspec(uuid("3071e40d-a10d-45ff-a59f-6e8e1138e2c1"))
friend class AXPlatformNodeTextProviderTest;
friend class AXRangeScreenRectDelegateImpl;
static bool AtStartOfLinePredicate(const AXPositionInstance& position);
static bool AtEndOfLinePredicate(const AXPositionInstance& position);
base::string16 GetString(int max_count);
AXPlatformNodeWin* owner() const;
AXPlatformNodeDelegate* GetDelegate(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment