diff --git a/accessible/basetypes/HyperTextAccessibleBase.cpp b/accessible/basetypes/HyperTextAccessibleBase.cpp index edfbae27011d..e12ce35e4e5a 100644 --- a/accessible/basetypes/HyperTextAccessibleBase.cpp +++ b/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 - // that might result in an infinite loop. - if (point <= endPoint) { + // before startPoint. In that case, we shouldn't try to move further forward, + // as that might result in an infinite loop. + 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 ok = false; diff --git a/accessible/tests/browser/hittest/browser_test_text.js b/accessible/tests/browser/hittest/browser_test_text.js index 1bd314e438c8..1ceb3beb81d4 100644 --- a/accessible/tests/browser/hittest/browser_test_text.js +++ b/accessible/tests/browser/hittest/browser_test_text.js @@ -13,6 +13,13 @@ a a + `, 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,