Skip to content

Commit

Permalink
[uia] ScrollIntoView causing scroll even when element is onscreen.
Browse files Browse the repository at this point in the history
This CL fixes a bug where calling ScrollIntoView on a fully onscreen
text range will cause it to move and align to the top of the view.
So we simply added a check so that if both of the text range's anchors
are onscreen, we return early.
Documentation:
https://learn.microsoft.com/en-us/windows/win32/api/uiautomationcore/nf-uiautomationcore-itextrangeprovider-scrollintoview

This CL also fixes a small bug in TestAXNodeWrapper::GetNodeFromID
that caused the function to only build wrappers for the passed in
node's children, rather than for all the nodes in the tree.

Change-Id: I9e3af9a22e968a1baf89e35e0b966deb48c7f557
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4347658
Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1118957}
  • Loading branch information
Javier Contreras Tenorio authored and Chromium LUCI CQ committed Mar 18, 2023
1 parent 4e20313 commit e4c9bb3
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 1 deletion.
Expand Up @@ -1019,6 +1019,19 @@ HRESULT AXPlatformNodeTextRangeProviderWin::ScrollIntoView(BOOL align_to_top) {
WIN_ACCESSIBILITY_API_HISTOGRAM(UMA_API_TEXTRANGE_SCROLLINTOVIEW);
UIA_VALIDATE_TEXTRANGEPROVIDER_CALL();

AXPlatformNode* start_platform_node =
GetOwner()->GetDelegate()->GetFromTreeIDAndNodeID(
start()->tree_id(), start()->GetAnchor()->id());
AXPlatformNode* end_platform_node =
GetOwner()->GetDelegate()->GetFromTreeIDAndNodeID(
end()->tree_id(), end()->GetAnchor()->id());

// If both anchors are onscreen, don't scroll.
if (!start_platform_node->GetDelegate()->IsOffscreen() &&
!end_platform_node->GetDelegate()->IsOffscreen()) {
return S_OK;
}

const AXPositionInstance start_common_ancestor =
start()->LowestCommonAncestorPosition(
*end(), ax::mojom::MoveDirection::kBackward);
Expand Down
Expand Up @@ -5233,6 +5233,39 @@ TEST_F(AXPlatformNodeTextRangeProviderTest,
EXPECT_EQ(end_offset, selection.focus_offset);
}

TEST_F(AXPlatformNodeTextRangeProviderTest,
TestScrollIntoViewOnOnscreenElement) {
TestAXTreeUpdate update(std::string(R"HTML(
++1 kRootWebArea
++++2 kGenericContainer
++++++3 kTextField
)HTML"));
update.nodes[0].relative_bounds.bounds = gfx::RectF(0, 0, 200, 200);
update.nodes[1].relative_bounds.bounds = gfx::RectF(0, 0, 100, 100);
update.nodes[2].SetValue("hello world test");
update.nodes[2].relative_bounds.bounds = gfx::RectF(50, 50, 20, 20);
update.nodes[2].relative_bounds.offset_container_id = 1;

Init(update);
AXNode* root_node = GetRoot();

AXNode* gc = root_node->children()[0];
AXNode* text_field = gc->children()[0];
ComPtr<ITextRangeProvider> text_range_provider;
GetTextRangeProviderFromTextNode(text_range_provider, text_field);

EXPECT_HRESULT_SUCCEEDED(
text_range_provider->ScrollIntoView(/* align_to_top */ true));
base::win::ScopedSafearray rectangles;
text_range_provider->GetBoundingRectangles(rectangles.Receive());

// Element was already fully onscreen, so there should be no change
// to its location.
std::vector<double> expected_rect = {50, 50, 20, 20};
EXPECT_UIA_SAFEARRAY_EQ(rectangles.Get(), expected_rect);
EXPECT_EQ(50, text_field->data().relative_bounds.bounds.y());
}

// TODO(crbug.com/1124051): Remove this test once this crbug is fixed.
TEST_F(AXPlatformNodeTextRangeProviderTest,
TestITextRangeProviderSelectListMarker) {
Expand Down
3 changes: 2 additions & 1 deletion ui/accessibility/platform/test_ax_node_wrapper.cc
Expand Up @@ -352,6 +352,7 @@ bool TestAXNodeWrapper::IsReadOnlyOrDisabled() const {

// Walk the AXTree and ensure that all wrappers are created
void TestAXNodeWrapper::BuildAllWrappers(AXTree* tree, AXNode* node) {
TestAXNodeWrapper::GetOrCreate(tree, node);
for (auto* child : node->children()) {
TestAXNodeWrapper::GetOrCreate(tree, child);
BuildAllWrappers(tree, child);
Expand All @@ -364,7 +365,7 @@ void TestAXNodeWrapper::ResetNativeEventTarget() {

AXPlatformNode* TestAXNodeWrapper::GetFromNodeID(int32_t id) {
// Force creating all of the wrappers for this tree.
BuildAllWrappers(tree_, node_);
BuildAllWrappers(tree_, tree_->root());

const auto iter = g_node_id_to_wrapper_map.find(id);
if (iter != g_node_id_to_wrapper_map.end())
Expand Down

0 comments on commit e4c9bb3

Please sign in to comment.