From a68d0f55b40f0f9b87cf82046c368f9499879a6c Mon Sep 17 00:00:00 2001 From: Morgan Rae Reschenberg Date: Wed, 29 Jun 2022 19:00:14 +0000 Subject: [PATCH] Bug 1775329: Support cached OffsetAtPoint in HyperTextAccessibleBase r=Jamie Differential Revision: https://phabricator.services.mozilla.com/D150049 --- accessible/base/TextLeafRange.cpp | 12 +- accessible/base/TextLeafRange.h | 9 + .../basetypes/HyperTextAccessibleBase.cpp | 27 +++ .../basetypes/HyperTextAccessibleBase.h | 5 + accessible/generic/HyperTextAccessible.h | 2 +- accessible/ipc/RemoteAccessibleShared.h | 3 +- accessible/ipc/other/RemoteAccessible.cpp | 6 + accessible/ipc/win/RemoteAccessible.cpp | 5 + accessible/tests/browser/e10s/browser.ini | 1 - .../browser/e10s/browser_caching_hittest.js | 187 ------------------ .../browser/hittest/browser_test_general.js | 22 +-- .../hittest/browser_test_shadowroot.js | 4 +- .../browser/hittest/browser_test_zoom_text.js | 10 +- accessible/tests/browser/hittest/head.js | 33 ++-- 14 files changed, 98 insertions(+), 228 deletions(-) delete mode 100644 accessible/tests/browser/e10s/browser_caching_hittest.js diff --git a/accessible/base/TextLeafRange.cpp b/accessible/base/TextLeafRange.cpp index e891921b4ba6..15260cb9fdc8 100644 --- a/accessible/base/TextLeafRange.cpp +++ b/accessible/base/TextLeafRange.cpp @@ -1403,7 +1403,7 @@ TextLeafPoint TextLeafPoint::FindTextAttrsStart(nsDirection aDirection, LayoutDeviceIntRect TextLeafPoint::CharBounds() { if (mAcc && !mAcc->IsText()) { - // If we're dealing with an empty embedded object, return the + // If we're dealing with an empty container, return the // accessible's non-text bounds. return mAcc->Bounds(); } @@ -1426,4 +1426,14 @@ LayoutDeviceIntRect TextLeafPoint::CharBounds() { return LayoutDeviceIntRect(); } +bool TextLeafPoint::ContainsPoint(int32_t aX, int32_t aY) { + if (mAcc && !mAcc->IsText()) { + // If we're dealing with an empty embedded object, use the + // accessible's non-text bounds. + return mAcc->Bounds().Contains(aX, aY); + } + + return CharBounds().Contains(aX, aY); +} + } // namespace mozilla::a11y diff --git a/accessible/base/TextLeafRange.h b/accessible/base/TextLeafRange.h index efaf89e53b46..f368dcb9e300 100644 --- a/accessible/base/TextLeafRange.h +++ b/accessible/base/TextLeafRange.h @@ -168,6 +168,15 @@ class TextLeafPoint final { */ LayoutDeviceIntRect CharBounds(); + /** + * Returns true if the given point (in screen coords) is contained + * in the char bounds of the current TextLeafPoint. Returns false otherwise. + * If the current point is an empty container, we use the acc's bounds instead + * of char bounds. Because this depends on CharBounds, this function only + * works on remote accessibles, and assumes caching is enabled. + */ + bool ContainsPoint(int32_t aX, int32_t aY); + bool IsLineFeedChar() const { return GetChar() == '\n'; } bool IsSpace() const; diff --git a/accessible/basetypes/HyperTextAccessibleBase.cpp b/accessible/basetypes/HyperTextAccessibleBase.cpp index 6cc51c4e50f9..9bb196b92015 100644 --- a/accessible/basetypes/HyperTextAccessibleBase.cpp +++ b/accessible/basetypes/HyperTextAccessibleBase.cpp @@ -247,6 +247,33 @@ LayoutDeviceIntRect HyperTextAccessibleBase::TextBounds(int32_t aStartOffset, return result; } +int32_t HyperTextAccessibleBase::OffsetAtPoint(int32_t aX, int32_t aY, + uint32_t aCoordType) { + Accessible* thisAcc = Acc(); + LayoutDeviceIntPoint coords = + nsAccUtils::ConvertToScreenCoords(aX, aY, aCoordType, thisAcc); + if (!thisAcc->Bounds().Contains(coords.x, coords.y)) { + // The requested point does not exist in this accessible. + return -1; + } + + TextLeafPoint point = 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 = + ToTextLeafPoint(static_cast(CharacterCount()), true); + endPoint = + endPoint.FindBoundary(nsIAccessibleText::BOUNDARY_CHAR, eDirPrevious, + /* aIncludeOrigin */ false); + // XXX: We should create a TextLeafRange object for this hypertext and move + // this search inside the TextLeafRange class. + for (; !point.ContainsPoint(coords.x, coords.y) && point != endPoint; + point = point.FindBoundary(nsIAccessibleText::BOUNDARY_CHAR, eDirNext, + /* aIncludeOrigin */ false)) { + }; + return point.ContainsPoint(coords.x, coords.y) ? point.mOffset : -1; +} + TextLeafPoint HyperTextAccessibleBase::ToTextLeafPoint(int32_t aOffset, bool aDescendToEnd) { Accessible* thisAcc = Acc(); diff --git a/accessible/basetypes/HyperTextAccessibleBase.h b/accessible/basetypes/HyperTextAccessibleBase.h index aea421fdd94e..689442abc6b2 100644 --- a/accessible/basetypes/HyperTextAccessibleBase.h +++ b/accessible/basetypes/HyperTextAccessibleBase.h @@ -115,6 +115,11 @@ class HyperTextAccessibleBase { int32_t aEndOffset, uint32_t aCoordType); + /** + * Return the offset of the char that contains the given coordinates. + */ + virtual int32_t OffsetAtPoint(int32_t aX, int32_t aY, uint32_t aCoordType); + /** * Get a TextLeafPoint for a given offset in this HyperTextAccessible. * If the offset points to an embedded object and aDescendToEnd is true, diff --git a/accessible/generic/HyperTextAccessible.h b/accessible/generic/HyperTextAccessible.h index 1e2de2ef376e..4491a784bf66 100644 --- a/accessible/generic/HyperTextAccessible.h +++ b/accessible/generic/HyperTextAccessible.h @@ -171,7 +171,7 @@ class HyperTextAccessible : public AccessibleWrap, /** * Return an offset at the given point. */ - int32_t OffsetAtPoint(int32_t aX, int32_t aY, uint32_t aCoordType); + int32_t OffsetAtPoint(int32_t aX, int32_t aY, uint32_t aCoordType) override; LayoutDeviceIntRect TextBounds( int32_t aStartOffset, int32_t aEndOffset, diff --git a/accessible/ipc/RemoteAccessibleShared.h b/accessible/ipc/RemoteAccessibleShared.h index 2bfce2ad593d..95d078a3c706 100644 --- a/accessible/ipc/RemoteAccessibleShared.h +++ b/accessible/ipc/RemoteAccessibleShared.h @@ -90,7 +90,8 @@ virtual void TextBeforeOffset(int32_t aOffset, char16_t CharAt(int32_t aOffset); -int32_t OffsetAtPoint(int32_t aX, int32_t aY, uint32_t aCoordType); +virtual int32_t OffsetAtPoint(int32_t aX, int32_t aY, + uint32_t aCoordType) override; bool SetSelectionBoundsAt(int32_t aSelectionNum, int32_t aStartOffset, int32_t aEndOffset); diff --git a/accessible/ipc/other/RemoteAccessible.cpp b/accessible/ipc/other/RemoteAccessible.cpp index 18e747f3b379..263cf9a6212a 100644 --- a/accessible/ipc/other/RemoteAccessible.cpp +++ b/accessible/ipc/other/RemoteAccessible.cpp @@ -309,6 +309,12 @@ LayoutDeviceIntRect RemoteAccessible::CharBounds(int32_t aOffset, int32_t RemoteAccessible::OffsetAtPoint(int32_t aX, int32_t aY, uint32_t aCoordType) { + if (StaticPrefs::accessibility_cache_enabled_AtStartup()) { + MOZ_ASSERT(IsHyperText(), "is not hypertext?"); + return RemoteAccessibleBase::OffsetAtPoint(aX, aY, + aCoordType); + } + int32_t retVal = -1; Unused << mDoc->SendOffsetAtPoint(mID, aX, aY, aCoordType, &retVal); return retVal; diff --git a/accessible/ipc/win/RemoteAccessible.cpp b/accessible/ipc/win/RemoteAccessible.cpp index e1d1134b1a8f..15b25ebb62ea 100644 --- a/accessible/ipc/win/RemoteAccessible.cpp +++ b/accessible/ipc/win/RemoteAccessible.cpp @@ -517,6 +517,11 @@ static IA2TextBoundaryType GetIA2TextBoundary( int32_t RemoteAccessible::OffsetAtPoint(int32_t aX, int32_t aY, uint32_t aCoordinateType) { + if (StaticPrefs::accessibility_cache_enabled_AtStartup()) { + return RemoteAccessibleBase::OffsetAtPoint( + aX, aY, aCoordinateType); + } + RefPtr acc = QueryInterface(this); if (!acc) { return -1; diff --git a/accessible/tests/browser/e10s/browser.ini b/accessible/tests/browser/e10s/browser.ini index b14e6c5291c6..14cad59e405a 100644 --- a/accessible/tests/browser/e10s/browser.ini +++ b/accessible/tests/browser/e10s/browser.ini @@ -72,4 +72,3 @@ skip-if = true # Failing due to incorrect index of test container children on do [browser_obj_group.js] skip-if = os == 'win' # Only supported with cache enabled [browser_caching_position.js] -[browser_caching_hittest.js] diff --git a/accessible/tests/browser/e10s/browser_caching_hittest.js b/accessible/tests/browser/e10s/browser_caching_hittest.js deleted file mode 100644 index 2468807abd35..000000000000 --- a/accessible/tests/browser/e10s/browser_caching_hittest.js +++ /dev/null @@ -1,187 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -"use strict"; - -const { Layout } = ChromeUtils.import( - "chrome://mochitests/content/browser/accessible/tests/browser/Layout.jsm" -); - -const { CommonUtils } = ChromeUtils.import( - "chrome://mochitests/content/browser/accessible/tests/browser/Common.jsm" -); - -function getChildAtPoint(container, x, y, findDeepestChild) { - try { - const child = findDeepestChild - ? container.getDeepestChildAtPoint(x, y) - : container.getChildAtPoint(x, y); - info(`Got child with role: ${roleToString(child.role)}`); - return child; - } catch (e) { - // Failed to get child at point. - } - - return null; -} - -async function testChildAtPoint(dpr, x, y, container, child, grandChild) { - const [containerX, containerY] = Layout.getBounds(container, dpr); - x += containerX; - y += containerY; - await untilCacheIs( - () => getChildAtPoint(container, x, y, false), - child, - `Wrong direct child accessible at the point (${x}, ${y}) of ${CommonUtils.prettyName( - container - )}, sought ${child ? roleToString(child.role) : "unknown"}` - ); - await untilCacheIs( - () => getChildAtPoint(container, x, y, true), - grandChild, - `Wrong deepest child accessible at the point (${x}, ${y}) of ${CommonUtils.prettyName( - container - )}, sought ${grandChild ? roleToString(grandChild.role) : "unknown"}` - ); -} - -async function hitTest(browser, container, child, grandChild) { - const [childX, childY] = await getContentBoundsForDOMElm( - browser, - getAccessibleDOMNodeID(child) - ); - const x = childX + 1; - const y = childY + 1; - - await untilCacheIs( - () => getChildAtPoint(container, x, y, false), - child, - `Wrong direct child accessible at the point (${x}, ${y}) of ${CommonUtils.prettyName( - container - )}, sought ${child ? roleToString(child.role) : "unknown"}` - ); - await untilCacheIs( - () => getChildAtPoint(container, x, y, true), - grandChild, - `Wrong deepest child accessible at the point (${x}, ${y}) of ${CommonUtils.prettyName( - container - )}, sought ${grandChild ? roleToString(grandChild.role) : "unknown"}` - ); -} - -async function runTests(browser, accDoc) { - await waitForImageMap(browser, accDoc); - const dpr = await getContentDPR(browser); - - await testChildAtPoint( - dpr, - 3, - 3, - findAccessibleChildByID(accDoc, "list"), - findAccessibleChildByID(accDoc, "listitem"), - findAccessibleChildByID(accDoc, "inner").firstChild - ); - todo( - false, - "Bug 746974 - children must match on all platforms. On Windows, " + - "ChildAtPoint with eDeepestChild is incorrectly ignoring MustPrune " + - "for the graphic." - ); - - const txt = findAccessibleChildByID(accDoc, "txt"); - await testChildAtPoint(dpr, 1, 1, txt, txt, txt); - - info( - "::MustPrune case, point is outside of textbox accessible but is in document." - ); - await testChildAtPoint(dpr, -1, -1, txt, null, null); - - info("::MustPrune case, point is outside of root accessible."); - await testChildAtPoint(dpr, -10000, -10000, txt, null, null); - - info("Not specific case, point is inside of btn accessible."); - const btn = findAccessibleChildByID(accDoc, "btn"); - await testChildAtPoint(dpr, 1, 1, btn, btn, btn); - - info("Not specific case, point is outside of btn accessible."); - await testChildAtPoint(dpr, -1, -1, btn, null, null); - - info( - "Out of flow accessible testing, do not return out of flow accessible " + - "because it's not a child of the accessible even though visually it is." - ); - await invokeContentTask(browser, [], () => { - // We have to reimprot CommonUtils in this scope -- eslint thinks this is - // wrong, but if you remove it, things will break. - /* eslint-disable no-shadow */ - const { CommonUtils } = ChromeUtils.import( - "chrome://mochitests/content/browser/accessible/tests/browser/Common.jsm" - ); - /* eslint-enable no-shadow */ - const doc = content.document; - const rectArea = CommonUtils.getNode("area", doc).getBoundingClientRect(); - const outOfFlow = CommonUtils.getNode("outofflow", doc); - outOfFlow.style.left = rectArea.left + "px"; - outOfFlow.style.top = rectArea.top + "px"; - }); - - const area = findAccessibleChildByID(accDoc, "area"); - await testChildAtPoint(dpr, 1, 1, area, area, area); - - info("Test image maps. Their children are not in the layout tree."); - const imgmap = findAccessibleChildByID(accDoc, "imgmap"); - const theLetterA = imgmap.firstChild; - await hitTest(browser, imgmap, theLetterA, theLetterA); - await hitTest( - browser, - findAccessibleChildByID(accDoc, "container"), - imgmap, - theLetterA - ); - - info("hit testing for element contained by zero-width element"); - const container2Input = findAccessibleChildByID(accDoc, "container2_input"); - await hitTest( - browser, - findAccessibleChildByID(accDoc, "container2"), - container2Input, - container2Input - ); -} - -addAccessibleTask( - ` -
-
inneritem
-
- - button1button2 - - textbox1textbox2 - -
-
-
- - - thelettera - - -
- -
- -
- -
- `, - runTests, - { - iframe: true, - remoteIframe: true, - // Ensure that all hittest elements are in view. - iframeAttrs: { style: "width: 600px; height: 600px; padding: 10px;" }, - } -); diff --git a/accessible/tests/browser/hittest/browser_test_general.js b/accessible/tests/browser/hittest/browser_test_general.js index 08596529ee76..97a3d76eb704 100644 --- a/accessible/tests/browser/hittest/browser_test_general.js +++ b/accessible/tests/browser/hittest/browser_test_general.js @@ -8,8 +8,7 @@ async function runTests(browser, accDoc) { await waitForImageMap(browser, accDoc); const dpr = await getContentDPR(browser); - info("Not specific case, child and deepchild testing."); - testChildAtPoint( + await testChildAtPoint( dpr, 3, 3, @@ -24,27 +23,23 @@ async function runTests(browser, accDoc) { "for the graphic." ); - info( - "::MustPrune case (in this case childAtPoint doesn't look inside a " + - "textbox), point is inside of textbox." - ); const txt = findAccessibleChildByID(accDoc, "txt"); - testChildAtPoint(dpr, 1, 1, txt, txt, txt); + await testChildAtPoint(dpr, 1, 1, txt, txt, txt); info( "::MustPrune case, point is outside of textbox accessible but is in document." ); - testChildAtPoint(dpr, -1, -1, txt, null, null); + await testChildAtPoint(dpr, -1, -1, txt, null, null); info("::MustPrune case, point is outside of root accessible."); - testChildAtPoint(dpr, -10000, -10000, txt, null, null); + await testChildAtPoint(dpr, -10000, -10000, txt, null, null); info("Not specific case, point is inside of btn accessible."); const btn = findAccessibleChildByID(accDoc, "btn"); - testChildAtPoint(dpr, 1, 1, btn, btn, btn); + await testChildAtPoint(dpr, 1, 1, btn, btn, btn); info("Not specific case, point is outside of btn accessible."); - testChildAtPoint(dpr, -1, -1, btn, null, null); + await testChildAtPoint(dpr, -1, -1, btn, null, null); info( "Out of flow accessible testing, do not return out of flow accessible " + @@ -54,7 +49,6 @@ async function runTests(browser, accDoc) { const { CommonUtils } = ChromeUtils.import( "chrome://mochitests/content/browser/accessible/tests/browser/Common.jsm" ); - const doc = content.document; const rectArea = CommonUtils.getNode("area", doc).getBoundingClientRect(); const outOfFlow = CommonUtils.getNode("outofflow", doc); @@ -63,7 +57,7 @@ async function runTests(browser, accDoc) { }); const area = findAccessibleChildByID(accDoc, "area"); - testChildAtPoint(dpr, 1, 1, area, area, area); + await testChildAtPoint(dpr, 1, 1, area, area, area); info("Test image maps. Their children are not in the layout tree."); const imgmap = findAccessibleChildByID(accDoc, "imgmap"); @@ -118,6 +112,6 @@ addAccessibleTask( iframe: true, remoteIframe: true, // Ensure that all hittest elements are in view. - iframeAttrs: { style: "width: 600px; height: 600px;" }, + iframeAttrs: { style: "width: 600px; height: 600px; padding: 10px;" }, } ); diff --git a/accessible/tests/browser/hittest/browser_test_shadowroot.js b/accessible/tests/browser/hittest/browser_test_shadowroot.js index cb499a4fd9fc..94a5ce071aa6 100644 --- a/accessible/tests/browser/hittest/browser_test_shadowroot.js +++ b/accessible/tests/browser/hittest/browser_test_shadowroot.js @@ -7,7 +7,7 @@ async function runTests(browser, accDoc) { const dpr = await getContentDPR(browser); let componentAcc = findAccessibleChildByID(accDoc, "component1"); - testChildAtPoint( + await testChildAtPoint( dpr, 1, 1, @@ -17,7 +17,7 @@ async function runTests(browser, accDoc) { ); componentAcc = findAccessibleChildByID(accDoc, "component2"); - testChildAtPoint( + await testChildAtPoint( dpr, 1, 1, diff --git a/accessible/tests/browser/hittest/browser_test_zoom_text.js b/accessible/tests/browser/hittest/browser_test_zoom_text.js index 925cd2c742cc..5c8a967cf775 100644 --- a/accessible/tests/browser/hittest/browser_test_zoom_text.js +++ b/accessible/tests/browser/hittest/browser_test_zoom_text.js @@ -7,9 +7,9 @@ /** * Test if getOffsetAtPoint returns the given text offset at given coordinates. */ -function testOffsetAtPoint(hyperText, x, y, coordType, expectedOffset) { - is( - hyperText.getOffsetAtPoint(x, y, coordType), +async function testOffsetAtPoint(hyperText, x, y, coordType, expectedOffset) { + await untilCacheIs( + () => hyperText.getOffsetAtPoint(x, y, coordType), expectedOffset, `Wrong offset at given point (${x}, ${y}) for ${prettyName(hyperText)}` ); @@ -33,7 +33,7 @@ async function runTests(browser, accDoc) { await getContentDPR(browser) ); - testOffsetAtPoint( + await testOffsetAtPoint( hyperText, x + width / 2, y + height / 2, @@ -55,7 +55,7 @@ async function runTests(browser, accDoc) { await getContentDPR(browser) ); - testOffsetAtPoint( + await testOffsetAtPoint( hyperText, x + width / 2, y + height / 2, diff --git a/accessible/tests/browser/hittest/head.js b/accessible/tests/browser/hittest/head.js index f06467da7cdc..1510a27c011b 100644 --- a/accessible/tests/browser/hittest/head.js +++ b/accessible/tests/browser/hittest/head.js @@ -41,25 +41,23 @@ function getChildAtPoint(container, x, y, findDeepestChild) { return null; } -function testChildAtPoint(dpr, x, y, container, child, grandChild) { +async function testChildAtPoint(dpr, x, y, container, child, grandChild) { const [containerX, containerY] = Layout.getBounds(container, dpr); x += containerX; y += containerY; - - CommonUtils.isObject( - getChildAtPoint(container, x, y, false), + await untilCacheIs( + () => getChildAtPoint(container, x, y, false), child, `Wrong direct child accessible at the point (${x}, ${y}) of ${CommonUtils.prettyName( container - )}` + )}, sought ${child ? roleToString(child.role) : "unknown"}` ); - - CommonUtils.isObject( - getChildAtPoint(container, x, y, true), + await untilCacheIs( + () => getChildAtPoint(container, x, y, true), grandChild, `Wrong deepest child accessible at the point (${x}, ${y}) of ${CommonUtils.prettyName( container - )}` + )}, sought ${grandChild ? roleToString(grandChild.role) : "unknown"}` ); } @@ -75,15 +73,18 @@ async function hitTest(browser, container, child, grandChild) { const x = childX + 1; const y = childY + 1; - CommonUtils.isObject( - getChildAtPoint(container, x, y, false), + await untilCacheIs( + () => getChildAtPoint(container, x, y, false), child, - `Wrong direct child of ${prettyName(container)}` + `Wrong direct child accessible at the point (${x}, ${y}) of ${CommonUtils.prettyName( + container + )}, sought ${child ? roleToString(child.role) : "unknown"}` ); - - CommonUtils.isObject( - getChildAtPoint(container, x, y, true), + await untilCacheIs( + () => getChildAtPoint(container, x, y, true), grandChild, - `Wrong deepest child of ${prettyName(container)}` + `Wrong deepest child accessible at the point (${x}, ${y}) of ${CommonUtils.prettyName( + container + )}, sought ${grandChild ? roleToString(grandChild.role) : "unknown"}` ); }