Fix: AXRange::GetScreenRects to ignore offscreen rects

Fix for AXRange::GetScreenRects to not return bounding boxes for ranges
that are offscreen.
Previously, we assumed ranges that are offscreen will have empty
bounding rects, this is incorrect because offscreen rects has its
width/height clipped to 1 rather than 0; so bounding box is not empty.
See doc:
https://cs.chromium.org/chromium/src/docs/accessibility/offscreen.md

Instead, GetScreenRects now looks at the offscreen status from the
bounds API to ignore offscreen ranges.

This fix is one of a series of fixes need for bug:964078.

Bug: 964078
Change-Id: Ibfb4d245845b7e18d59724b8c7fd09f9cc3de793
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824061
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#702893}

ui/accessibility/ax_range.h

@@ -11,6 +11,7 @@
 
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "ui/accessibility/ax_offscreen_result.h"
 #include "ui/accessibility/ax_role_properties.h"
 #include "ui/accessibility/ax_tree_manager_map.h"
 
@@ -27,11 +28,15 @@ enum class AXTextConcatenationBehavior {
 
 class AXRangeScreenRectDelegate {
  public:
-  virtual gfx::Rect GetInnerTextRangeBoundsRect(AXTreeID tree_id,
+  virtual gfx::Rect GetInnerTextRangeBoundsRect(
+      AXTreeID tree_id,
       AXNode::AXID node_id,
       int start_offset,
-                                                int end_offset) = 0;
-  virtual gfx::Rect GetBoundsRect(AXTreeID tree_id, AXNode::AXID node_id) = 0;
+      int end_offset,
+      AXOffscreenResult* offscreen_result) = 0;
+  virtual gfx::Rect GetBoundsRect(AXTreeID tree_id,
+                                  AXNode::AXID node_id,
+                                  AXOffscreenResult* offscreen_result) = 0;
 };
 
 // A range delimited by two positions in the AXTree.
@@ -304,21 +309,29 @@ class AXRange {
       // For text anchors, we retrieve the bounding rectangles of its text
       // content. For non-text anchors (such as checkboxes, images, etc.), we
       // want to directly retrieve their bounding rectangles.
+      AXOffscreenResult offscreen_result;
       gfx::Rect current_rect =
           (current_line_start->IsInLineBreak() ||
            current_line_start->IsInTextObject())
               ? delegate->GetInnerTextRangeBoundsRect(
                     current_tree_id, current_line_start->anchor_id(),
                     current_line_start->text_offset(),
-                    current_line_end->text_offset())
+                    current_line_end->text_offset(), &offscreen_result)
               : delegate->GetBoundsRect(current_tree_id,
-                                        current_line_start->anchor_id());
-
-      // We only add rects that are visible within the current viewport.
-      // If the bounding rectangle is outside the viewport, the kClipped
-      // parameter from the bounds APIs will result in returning an empty
-      // rect, which we should omit from the final result.
-      if (!current_rect.IsEmpty())
+                                        current_line_start->anchor_id(),
+                                        &offscreen_result);
+
+      // We only add rects that do not represent a degenerate range and rects
+      // that are onscreen.
+      // If the represented range is degenerate, the bounding rectangles will be
+      // empty.
+      // If the represented range is offscreen, the bounding rectangles will be
+      // clipped, with its width/height set to 1. The bounding rectangles is not
+      // set to empty when it is offscreen.
+      // Documentation for offscreen bounding box size calculation:
+      // https://chromium.googlesource.com/chromium/src/+/HEAD/docs/accessibility/offscreen.md
+      if (!current_rect.IsEmpty() &&
+          offscreen_result == AXOffscreenResult::kOnscreen)
         rects.push_back(current_rect);
     }
     return rects;

ui/accessibility/ax_range_unittest.cc

@@ -47,10 +47,12 @@ class TestAXRangeScreenRectDelegate : public AXRangeScreenRectDelegate {
  public:
   TestAXRangeScreenRectDelegate(AXTree* tree) : tree_(tree) {}
 
-  gfx::Rect GetInnerTextRangeBoundsRect(AXTreeID tree_id,
+  gfx::Rect GetInnerTextRangeBoundsRect(
+      AXTreeID tree_id,
       AXNode::AXID node_id,
       int start_offset,
-                                        int end_offset) override {
+      int end_offset,
+      AXOffscreenResult* offscreen_result) override {
     if (tree_->data().tree_id != tree_id)
       return gfx::Rect();
 
@@ -60,13 +62,14 @@ class TestAXRangeScreenRectDelegate : public AXRangeScreenRectDelegate {
 
     TestAXNodeWrapper* wrapper = TestAXNodeWrapper::GetOrCreate(tree_, node);
 
-    AXOffscreenResult ignore_offscreen_result;
     return wrapper->GetInnerTextRangeBoundsRect(
         start_offset, end_offset, ui::AXCoordinateSystem::kScreen,
-        ui::AXClippingBehavior::kClipped, &ignore_offscreen_result);
+        ui::AXClippingBehavior::kClipped, offscreen_result);
   }
 
-  gfx::Rect GetBoundsRect(AXTreeID tree_id, AXNode::AXID node_id) override {
+  gfx::Rect GetBoundsRect(AXTreeID tree_id,
+                          AXNode::AXID node_id,
+                          AXOffscreenResult* offscreen_result) override {
     if (tree_->data().tree_id != tree_id)
       return gfx::Rect();
 
@@ -75,10 +78,9 @@ class TestAXRangeScreenRectDelegate : public AXRangeScreenRectDelegate {
       return gfx::Rect();
 
     TestAXNodeWrapper* wrapper = TestAXNodeWrapper::GetOrCreate(tree_, node);
-    AXOffscreenResult ignore_offscreen_result;
     return wrapper->GetBoundsRect(ui::AXCoordinateSystem::kScreen,
                                   ui::AXClippingBehavior::kClipped,
-                                  &ignore_offscreen_result);
+                                  offscreen_result);
   }
 
  private:
@@ -1236,4 +1238,54 @@ TEST_F(AXRangeTest, GetScreenRects) {
               testing::ContainerEq(expected_screen_rects));
 }
 
+TEST_F(AXRangeTest, GetScreenRectsOffscreen) {
+  // Set up root node bounds/viewport size  to {0, 50, 800x60}, so that only
+  // some text will be onscreen the rest will be offscreen.
+  AXNodeData old_root_node_data = GetRootNode()->data();
+  AXNodeData new_root_node_data = old_root_node_data;
+  new_root_node_data.relative_bounds.bounds = gfx::RectF(0, 50, 800, 60);
+  GetRootNode()->SetData(new_root_node_data);
+
+  TestAXRangeScreenRectDelegate delegate(tree_.get());
+
+  TestPositionInstance button = AXNodePosition::CreateTextPosition(
+      tree_->data().tree_id, button_.id, 0 /* text_offset */,
+      ax::mojom::TextAffinity::kDownstream);
+
+  TestPositionInstance after_line_end = AXNodePosition::CreateTextPosition(
+      tree_->data().tree_id, inline_box3_.id, 5 /* text_offset */,
+      ax::mojom::TextAffinity::kDownstream);
+
+  // [Button]           [Checkbox 1]         [Checkbox 2]
+  // {20, 20, 100x30},  {120, 20, 30x30}     {150, 20, 30x30}
+  //                                              ---
+  // [Line 1]                                     |
+  // {20, 50, 30x30}                              | view port, onscreen
+  //                                              | {0, 50, 800x60}
+  // [Line 2]                                     |
+  // {20, 80, 42x30}                              |
+  //                                              ---
+  // [After]
+  // {20, 110, 50x30}
+  //
+  // Retrieving bounding boxes of the range spanning the entire document.
+  // |[Button][Checkbox 1][Checkbox 2]L|i|n|e| |1|\n|L|i|n|e| |2|\n|A|f|t|e|r|
+  // |-----------------------------------------------------------------------|
+  TestPositionRange entire_test_range(button->Clone(), after_line_end->Clone());
+  std::vector<gfx::Rect> expected_screen_rects = {gfx::Rect(20, 50, 30, 30),
+                                                  gfx::Rect(20, 80, 42, 30)};
+  EXPECT_THAT(entire_test_range.GetScreenRects(&delegate),
+              testing::ContainerEq(expected_screen_rects));
+
+  // Reset the root node bounds/viewport size back to {0, 0, 800x600}, and
+  // verify all elements should be onscreen.
+  GetRootNode()->SetData(old_root_node_data);
+  expected_screen_rects = {
+      gfx::Rect(20, 20, 100, 30), gfx::Rect(120, 20, 30, 30),
+      gfx::Rect(150, 20, 30, 30), gfx::Rect(20, 50, 30, 30),
+      gfx::Rect(20, 80, 42, 30),  gfx::Rect(20, 110, 50, 30)};
+  EXPECT_THAT(entire_test_range.GetScreenRects(&delegate),
+              testing::ContainerEq(expected_screen_rects));
+}
+
 }  // namespace ui

ui/accessibility/platform/ax_platform_node_textrangeprovider_win.cc

@@ -30,22 +30,27 @@ class AXRangeScreenRectDelegateImpl : public AXRangeScreenRectDelegate {
   AXRangeScreenRectDelegateImpl(AXPlatformNodeTextRangeProviderWin* host)
       : host_(host) {}
 
-  gfx::Rect GetInnerTextRangeBoundsRect(AXTreeID tree_id,
+  gfx::Rect GetInnerTextRangeBoundsRect(
+      AXTreeID tree_id,
       AXNode::AXID node_id,
       int start_offset,
-                                        int end_offset) override {
+      int end_offset,
+      AXOffscreenResult* offscreen_result) override {
     AXPlatformNodeDelegate* delegate = host_->GetDelegate(tree_id, node_id);
     DCHECK(delegate);
     return delegate->GetInnerTextRangeBoundsRect(
         start_offset, end_offset, ui::AXCoordinateSystem::kScreen,
-        ui::AXClippingBehavior::kClipped);
+        ui::AXClippingBehavior::kClipped, offscreen_result);
   }
 
-  gfx::Rect GetBoundsRect(AXTreeID tree_id, AXNode::AXID node_id) override {
+  gfx::Rect GetBoundsRect(AXTreeID tree_id,
+                          AXNode::AXID node_id,
+                          AXOffscreenResult* offscreen_result) override {
     AXPlatformNodeDelegate* delegate = host_->GetDelegate(tree_id, node_id);
     DCHECK(delegate);
     return delegate->GetBoundsRect(ui::AXCoordinateSystem::kScreen,
-                                   ui::AXClippingBehavior::kClipped);
+                                   ui::AXClippingBehavior::kClipped,
+                                   offscreen_result);
   }
 
  private:

ui/accessibility/platform/test_ax_node_wrapper.cc

@@ -152,6 +152,14 @@ gfx::Rect TestAXNodeWrapper::GetBoundsRect(
       // We could optionally add clipping here if ever needed.
       gfx::RectF bounds = GetLocation();
       bounds.Offset(g_offset);
+
+      // For test behavior only, for bounds that are offscreen we currently do
+      // not apply clipping to the bounds but we still return the offscreen
+      // status.
+      if (offscreen_result) {
+        *offscreen_result = DetermineOffscreenResult(bounds);
+      }
+
       return gfx::ToEnclosingRect(bounds);
     }
     case AXCoordinateSystem::kRootFrame:
@@ -186,6 +194,14 @@ gfx::Rect TestAXNodeWrapper::GetInnerTextRangeBoundsRect(
       }
 
       bounds.Offset(g_offset);
+
+      // For test behavior only, for bounds that are offscreen we currently do
+      // not apply clipping to the bounds but we still return the offscreen
+      // status.
+      if (offscreen_result) {
+        *offscreen_result = DetermineOffscreenResult(bounds);
+      }
+
       return gfx::ToEnclosingRect(bounds);
     }
     case AXCoordinateSystem::kRootFrame:
@@ -838,4 +854,28 @@ gfx::RectF TestAXNodeWrapper::GetInlineTextRect(const int start_offset,
   return bounds;
 }
 
+AXOffscreenResult TestAXNodeWrapper::DetermineOffscreenResult(
+    gfx::RectF bounds) const {
+  if (!tree_ || !tree_->root())
+    return AXOffscreenResult::kOnscreen;
+
+  const AXNodeData& root_web_area_node_data = tree_->root()->data();
+  gfx::RectF root_web_area_bounds =
+      root_web_area_node_data.relative_bounds.bounds;
+
+  // For testing, we only look at the current node's bound relative to the root
+  // web area bounds to determine offscreen status. We currently do not look at
+  // the bounds of the immediate parent of the node for determining offscreen
+  // status.
+  // We only determine offscreen result if the root web area bounds is actually
+  // set in the test. We default the offscreen result of every other situation
+  // to AXOffscreenResult::kOnscreen.
+  if (!root_web_area_bounds.IsEmpty()) {
+    bounds.Intersect(root_web_area_bounds);
+    if (bounds.IsEmpty())
+      return AXOffscreenResult::kOffscreen;
+  }
+  return AXOffscreenResult::kOnscreen;
+}
+
 }  // namespace ui

ui/accessibility/platform/test_ax_node_wrapper.h

@@ -156,6 +156,9 @@ class TestAXNodeWrapper : public AXPlatformNodeDelegateBase {
   gfx::RectF GetInlineTextRect(const int start_offset,
                                const int end_offset) const;
 
+  // Determine the offscreen status of a particular element given its bounds..
+  AXOffscreenResult DetermineOffscreenResult(gfx::RectF bounds) const;
+
   AXTree* tree_;
   AXNode* node_;
   ui::AXUniqueId unique_id_;