Bug 1816601: HyperTextAccessibleBase::OffsetAtPoint: Return 0 if the point is within the container but before the rect at offset 0. r=nlapre

This perpetuates a bug in (local) HyperTextAccessible that some users have unfortunately come to rely on.

Differential Revision: https://phabricator.services.mozilla.com/D169802

accessible/basetypes/HyperTextAccessibleBase.cpp

@@ -279,7 +279,7 @@ int32_t HyperTextAccessibleBase::OffsetAtPoint(int32_t aX, int32_t aY,
     return -1;
   }
 
-  TextLeafPoint point = ToTextLeafPoint(0, false);
+  TextLeafPoint startPoint = ToTextLeafPoint(0, false);
   // As with TextBounds, we walk to the very end of the text contained in this
   // hypertext and then step backwards to make our endPoint inclusive.
   TextLeafPoint endPoint =
@@ -287,18 +287,27 @@ int32_t HyperTextAccessibleBase::OffsetAtPoint(int32_t aX, int32_t aY,
   endPoint =
       endPoint.FindBoundary(nsIAccessibleText::BOUNDARY_CHAR, eDirPrevious,
                             /* aIncludeOrigin */ false);
+  TextLeafPoint point = startPoint;
   // XXX: We should create a TextLeafRange object for this hypertext and move
   // this search inside the TextLeafRange class.
   // If there are no characters in this container, we might have moved endPoint
-  // before point. In that case, we shouldn't try to move further forward, as
+  // before startPoint. In that case, we shouldn't try to move further forward,
-  // that might result in an infinite loop.
+  // as that might result in an infinite loop.
-  if (point <= endPoint) {
+  if (startPoint <= endPoint) {
     for (; !point.ContainsPoint(coords.x, coords.y) && point != endPoint;
          point = point.FindBoundary(nsIAccessibleText::BOUNDARY_CHAR, eDirNext,
                                     /* aIncludeOrigin */ false)) {
     }
   }
   if (!point.ContainsPoint(coords.x, coords.y)) {
+    LayoutDeviceIntRect startRect = startPoint.CharBounds();
+    if (coords.x < startRect.x || coords.y < startRect.y) {
+      // Bug 1816601: The point is within the container but above or to the left
+      // of the rectangle at offset 0. We should really return -1, but we've
+      // returned 0 for many years due to a bug. Some users have unfortunately
+      // come to rely on this, so perpetuate this here.
+      return 0;
+    }
     return -1;
   }
   DebugOnly<bool> ok = false;

accessible/tests/browser/hittest/browser_test_text.js

@@ -13,6 +13,13 @@ a
   a
   <iframe width="1" height="1"></iframe>
 </div>
+<button id="pointBeforeText">
+  <div style="display: flex;">
+    <div style="width: 100px; background-color: red;" role="none"></div>
+    test
+    <div style="width: 100px; background-color: blue;" role="none"></div>
+  </div>
+</button>
   `,
   async function(browser, docAcc) {
     const dpr = await getContentDPR(browser);
@@ -43,6 +50,30 @@ a
     x += width - 1;
     y += height - 1;
     await testOffsetAtPoint(iframeAtEnd, x, y, COORDTYPE_SCREEN_RELATIVE, -1);
+
+    // Test that 0 is returned if the point is within the container but before
+    // the rectangle at offset 0. This is buggy behavior that some users have
+    // unfortunately come to rely on (bug 1816601).
+    const pointBeforeText = findAccessibleChildByID(docAcc, "pointBeforeText", [
+      Ci.nsIAccessibleText,
+    ]);
+    [x, y, width, height] = Layout.getBounds(pointBeforeText, dpr);
+    await testOffsetAtPoint(
+      pointBeforeText,
+      x + 1,
+      y + 1,
+      COORDTYPE_SCREEN_RELATIVE,
+      0
+    );
+    // But this buggy behavior only applies for a point before offset 0, not
+    // a point after the last offset. So it's asymmetrically buggy. :(
+    await testOffsetAtPoint(
+      pointBeforeText,
+      x + width - 1,
+      y + height - 1,
+      COORDTYPE_SCREEN_RELATIVE,
+      -1
+    );
   },
   {
     topLevel: true,