Scroll[Substring]ToPoint does not work with screen-relative coordinates

When screen relative coordinates are passed in, do not alter them, they are already correct. Note: compare with results in "Better failing test" patchset 3. Bug: 889491 Change-Id: I7618aab997f01cca20d6c712385e19beff331647 Reviewed-on: https://chromium-review.googlesource.com/1246321 Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org> Cr-Commit-Position: refs/heads/master@{#594779}
Denger-Network · Sep 27, 2018 · 1534ee3 · 1534ee3
1 parent c5ee8ad
commit 1534ee3
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 11 deletions.
diff --git a/content/browser/accessibility/accessibility_win_browsertest.cc b/content/browser/accessibility/accessibility_win_browsertest.cc
@@ -330,8 +330,9 @@ void AccessibilityWinBrowserTest::SetUpSampleParagraphInScrollableDocument(
   LoadInitialAccessibilityTreeFromHtml(
       R"HTML(<!DOCTYPE html>
       <html>
-      <body style="overflow: scroll; margin-top: 100vh">
-        <p><b>Game theory</b> is "the study of
+      <body>
+        <p style="margin-top:50vh; margin-bottom:200vh">
+            <b>Game theory</b> is "the study of
             <a href="" title="Mathematical model">mathematical models</a>
             of conflict and<br>cooperation between intelligent rational
             decision-makers."
@@ -1433,15 +1434,55 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest, TestScrollToPoint) {
   EXPECT_EQ(prev_x, x);
   EXPECT_GT(prev_y, y);
 
-  prev_x = x;
-  prev_y = y;
+  constexpr int kScrollToY = 0;
   EXPECT_HRESULT_SUCCEEDED(
-      paragraph->scrollToPoint(IA2_COORDTYPE_SCREEN_RELATIVE, 0, 0));
+      paragraph->scrollToPoint(IA2_COORDTYPE_SCREEN_RELATIVE, 0, kScrollToY));
   location_changed_waiter.WaitForNotification();
   ASSERT_HRESULT_SUCCEEDED(
       paragraph->accLocation(&x, &y, &width, &height, childid_self));
-  EXPECT_EQ(prev_x, x);
-  EXPECT_EQ(prev_y, y);
+  EXPECT_EQ(kScrollToY, y);
+
+  constexpr int kScrollToY_2 = 243;
+  EXPECT_HRESULT_SUCCEEDED(
+      paragraph->scrollToPoint(IA2_COORDTYPE_SCREEN_RELATIVE, 0, kScrollToY_2));
+  location_changed_waiter.WaitForNotification();
+  ASSERT_HRESULT_SUCCEEDED(
+      paragraph->accLocation(&x, &y, &width, &height, childid_self));
+  EXPECT_EQ(kScrollToY_2, y);
+}
+
+IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
+                       TestScrollSubstringToPoint) {
+  Microsoft::WRL::ComPtr<IAccessibleText> paragraph_text;
+  SetUpSampleParagraphInScrollableDocument(&paragraph_text);
+  Microsoft::WRL::ComPtr<IAccessible2> paragraph;
+  ASSERT_HRESULT_SUCCEEDED(paragraph_text.CopyTo(IID_PPV_ARGS(&paragraph)));
+
+  LONG x, y, width, height;
+  base::win::ScopedVariant childid_self(CHILDID_SELF);
+  AccessibilityNotificationWaiter location_changed_waiter(
+      shell()->web_contents(), ui::kAXModeComplete,
+      ax::mojom::Event::kLocationChanged);
+
+  constexpr int kScrollToY = 187;
+  constexpr int kCharOffset = 10;
+  EXPECT_HRESULT_SUCCEEDED(paragraph_text->scrollSubstringToPoint(
+      kCharOffset, kCharOffset + 1, IA2_COORDTYPE_SCREEN_RELATIVE, 0,
+      kScrollToY));
+  location_changed_waiter.WaitForNotification();
+  ASSERT_HRESULT_SUCCEEDED(paragraph_text->get_characterExtents(
+      kCharOffset, IA2_COORDTYPE_SCREEN_RELATIVE, &x, &y, &width, &height));
+  EXPECT_EQ(kScrollToY, y);
+
+  constexpr int kScrollToY_2 = -133;
+  constexpr int kCharOffset_2 = 30;
+  EXPECT_HRESULT_SUCCEEDED(paragraph_text->scrollSubstringToPoint(
+      kCharOffset_2, kCharOffset_2 + 1, IA2_COORDTYPE_SCREEN_RELATIVE, 0,
+      kScrollToY_2));
+  location_changed_waiter.WaitForNotification();
+  ASSERT_HRESULT_SUCCEEDED(paragraph_text->get_characterExtents(
+      kCharOffset_2, IA2_COORDTYPE_SCREEN_RELATIVE, &x, &y, &width, &height));
+  EXPECT_EQ(kScrollToY_2, y);
 }
 
 IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest, TestSetCaretOffset) {

diff --git a/ui/accessibility/platform/ax_platform_node_win.cc b/ui/accessibility/platform/ax_platform_node_win.cc
@@ -1367,16 +1367,15 @@ IFACEMETHODIMP AXPlatformNodeWin::scrollToPoint(
   COM_OBJECT_VALIDATE();
   WIN_ACCESSIBILITY_API_HISTOGRAM(UMA_API_SCROLL_TO_POINT);
 
+  // Convert to screen-relative coordinates if necessary.
   gfx::Point scroll_to(x, y);
-  if (coordinate_type == IA2_COORDTYPE_SCREEN_RELATIVE) {
-    scroll_to -= delegate_->GetUnclippedScreenBoundsRect().OffsetFromOrigin();
-  } else if (coordinate_type == IA2_COORDTYPE_PARENT_RELATIVE) {
+  if (coordinate_type == IA2_COORDTYPE_PARENT_RELATIVE) {
     if (GetParent()) {
       AXPlatformNodeBase* base = FromNativeViewAccessible(GetParent());
       scroll_to +=
           base->delegate_->GetUnclippedScreenBoundsRect().OffsetFromOrigin();
     }
-  } else {
+  } else if (coordinate_type != IA2_COORDTYPE_SCREEN_RELATIVE) {
     return E_INVALIDARG;
   }