Commit 50ae57a7 authored by Nathan LaPre's avatar Nathan LaPre
Browse files

Bug 1819741: Constrain acc bounds to that of scroll areas for hit testing, r=morgan 

This revision adds logic to BoundsWithOffset to ensure that bounds, when
calculated for hit testing, are constrained to the scroll areas that contain
them. This ensures that we don't return an Accessible that's covered by another
element when hit testing due to overflow: scroll situations.

This revision also contains a fix for transforms and scroll: we now apply
scroll before any transform, since transforms operate on scrolled content.

This revision contains tests for both of the above changes.

Differential Revision: https://phabricator.services.mozilla.com/D173392
parent e53a6264

accessible/ipc/RemoteAccessibleBase.cpp

+37 −9
Original line number Diff line number Diff line
@@ -331,7 +331,7 @@ double RemoteAccessibleBase<Derived>::Step() const { 


template <class Derived>

bool RemoteAccessibleBase<Derived>::ContainsPoint(int32_t aX, int32_t aY) {

  if (!Bounds().Contains(aX, aY)) {

  if (!BoundsWithOffset(Nothing(), true).Contains(aX, aY)) {

    return false;

  }

  if (!IsTextLeaf()) {
@@ -380,7 +380,7 @@ bool RemoteAccessibleBase<Derived>::ContainsPoint(int32_t aX, int32_t aY) { 
    if (lineEnd > lineStart) {

      lineRect.UnionRect(lineRect, GetCachedCharRect(lineEnd));

    }

    if (BoundsWithOffset(Some(lineRect)).Contains(aX, aY)) {

    if (BoundsWithOffset(Some(lineRect), true).Contains(aX, aY)) {

      return true;

    }

    lineStart = lineEnd + 1;
@@ -391,10 +391,16 @@ bool RemoteAccessibleBase<Derived>::ContainsPoint(int32_t aX, int32_t aY) { 
template <class Derived>

Accessible* RemoteAccessibleBase<Derived>::ChildAtPoint(

    int32_t aX, int32_t aY, LocalAccessible::EWhichChildAtPoint aWhichChild) {

  // Elements that are partially on-screen should have their bounds masked by

  // their containing scroll area so hittesting yields results that are

  // consistent with the content's visual representation. Pass this value to

  // bounds calculation functions to indicate that we're hittesting.

  const bool hitTesting = true;



  if (IsOuterDoc() && aWhichChild == EWhichChildAtPoint::DirectChild) {

    // This is an iframe, which is as deep as the viewport cache goes. The

    // caller wants a direct child, which can only be the embedded document.

    if (Bounds().Contains(aX, aY)) {

    if (BoundsWithOffset(Nothing(), hitTesting).Contains(aX, aY)) {

      return RemoteFirstChild();

    }

    return nullptr;
@@ -429,7 +435,7 @@ Accessible* RemoteAccessibleBase<Derived>::ChildAtPoint( 


        if (acc->IsOuterDoc() &&

            aWhichChild == EWhichChildAtPoint::DeepestChild &&

            acc->Bounds().Contains(aX, aY)) {

            acc->BoundsWithOffset(Nothing(), hitTesting).Contains(aX, aY)) {

          // acc is an iframe, which is as deep as the viewport cache goes. This

          // iframe contains the requested point.

          RemoteAccessible* innerDoc = acc->RemoteFirstChild();
@@ -492,7 +498,7 @@ Accessible* RemoteAccessibleBase<Derived>::ChildAtPoint( 
    lastMatch = nullptr;

  }



  if (!lastMatch && Bounds().Contains(aX, aY)) {

  if (!lastMatch && BoundsWithOffset(Nothing(), hitTesting).Contains(aX, aY)) {

    // Even though the hit target isn't inside `this`, the point is still

    // within our bounds, so fall back to `this`.

    return this;
@@ -572,12 +578,12 @@ bool RemoteAccessibleBase<Derived>::ApplyTransform( 
}



template <class Derived>

void RemoteAccessibleBase<Derived>::ApplyScrollOffset(nsRect& aBounds) const {

bool RemoteAccessibleBase<Derived>::ApplyScrollOffset(nsRect& aBounds) const {

  Maybe<const nsTArray<int32_t>&> maybeScrollPosition =

      mCachedFields->GetAttribute<nsTArray<int32_t>>(nsGkAtoms::scrollPosition);



  if (!maybeScrollPosition || maybeScrollPosition->Length() != 2) {

    return;

    return false;

  }

  // Our retrieved value is in app units, so we don't need to do any

  // unit conversion here.
@@ -589,6 +595,11 @@ void RemoteAccessibleBase<Derived>::ApplyScrollOffset(nsRect& aBounds) const { 
  nsPoint scrollOffset(-scrollPosition[0], -scrollPosition[1]);



  aBounds.MoveBy(scrollOffset.x, scrollOffset.y);



  // Return true here even if the scroll offset was 0,0 because the RV is used

  // as a scroll container indicator. Non-scroll containers won't have cached

  // scroll position.

  return true;

}



template <class Derived>
@@ -620,7 +631,7 @@ bool RemoteAccessibleBase<Derived>::IsFixedPos() const { 


template <class Derived>

LayoutDeviceIntRect RemoteAccessibleBase<Derived>::BoundsWithOffset(

    Maybe<nsRect> aOffset) const {

    Maybe<nsRect> aOffset, bool aBoundsAreForHittesting) const {

  Maybe<nsRect> maybeBounds = RetrieveCachedBounds();

  if (maybeBounds) {

    nsRect bounds = *maybeBounds;
@@ -647,6 +658,12 @@ LayoutDeviceIntRect RemoteAccessibleBase<Derived>::BoundsWithOffset( 
    const Accessible* acc = Parent();

    bool encounteredFixedContainer = IsFixedPos();

    while (acc && acc->IsRemote()) {

      // Return early if we're hit testing and our cumulative bounds are empty,

      // since walking the ancestor chain won't produce any hits.

      if (aBoundsAreForHittesting && bounds.IsEmpty()) {

        return LayoutDeviceIntRect{};

      }



      RemoteAccessible* remoteAcc = const_cast<Accessible*>(acc)->AsRemote();



      if (Maybe<nsRect> maybeRemoteBounds = remoteAcc->RetrieveCachedBounds()) {
@@ -682,7 +699,18 @@ LayoutDeviceIntRect RemoteAccessibleBase<Derived>::BoundsWithOffset( 
          // happens in this loop instead of both inside and outside of

          // the loop (like ApplyTransform).

          // Never apply scroll offsets past a fixed container.

          remoteAcc->ApplyScrollOffset(remoteBounds);

          const bool hasScrollArea = remoteAcc->ApplyScrollOffset(bounds);



          // If we are hit testing and the Accessible has a scroll area, ensure

          // that the bounds we've calculated so far are constrained to the

          // bounds of the scroll area. Without this, we'll "hit" the off-screen

          // portions of accs that are are partially (but not fully) within the

          // scroll area.

          if (aBoundsAreForHittesting && hasScrollArea) {

            nsRect selfRelativeScrollBounds(0, 0, remoteBounds.width,

                                            remoteBounds.height);

            bounds = bounds.SafeIntersect(selfRelativeScrollBounds);

          }

        }

        if (remoteAcc->IsDoc()) {

          // Fixed elements are document relative, so if we've hit a

accessible/ipc/RemoteAccessibleBase.h

+5 −2
Original line number Diff line number Diff line
@@ -436,10 +436,13 @@ class RemoteAccessibleBase : public Accessible, public HyperTextAccessibleBase { 
  void SetParent(Derived* aParent);

  Maybe<nsRect> RetrieveCachedBounds() const;

  bool ApplyTransform(nsRect& aCumulativeBounds) const;

  void ApplyScrollOffset(nsRect& aBounds) const;

  bool ApplyScrollOffset(nsRect& aBounds) const;

  void ApplyCrossDocOffset(nsRect& aBounds) const;

  LayoutDeviceIntRect BoundsWithOffset(Maybe<nsRect> aOffset) const;

  LayoutDeviceIntRect BoundsWithOffset(

      Maybe<nsRect> aOffset, bool aBoundsAreForHittesting = false) const;

  bool IsFixedPos() const;



  // This function is used exclusively for hit testing.

  bool ContainsPoint(int32_t aX, int32_t aY);



  virtual void ARIAGroupPosition(int32_t* aLevel, int32_t* aSetSize,

accessible/tests/browser/bounds/browser_test_simple_transform.js

+27 −0
Original line number Diff line number Diff line
@@ -196,3 +196,30 @@ addAccessibleTask( 
  },

  { topLevel: true, iframe: true, remoteIframe: true }

);



// Test bounds of a rotated element after scroll.

addAccessibleTask(

  `

<div id="scrollable" style="transform: rotate(180deg); overflow: scroll; height: 500px;">

  <p id="test">hello world</p><hr style="height: 100vh;">

</div>

  `,

  async function(browser, docAcc) {

    info(

      "Testing that the unscrolled bounds of a transformed element are correct."

    );

    await testBoundsWithContent(docAcc, "test", browser);



    await invokeContentTask(browser, [], () => {

      // Scroll the scrollable region down (scrolls up due to the transform).

      let elem = content.document.getElementById("scrollable");

      elem.scrollTo(0, elem.scrollHeight);

    });



    info(

      "Testing that the scrolled bounds of a transformed element are correct."

    );

    await testBoundsWithContent(docAcc, "test", browser);

  },

  { topLevel: true, iframe: true, remoteIframe: true }

);

accessible/tests/browser/hittest/browser.ini

+1 −0
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ prefs = 


[browser_test_browser.js]

[browser_test_general.js]

[browser_test_scroll_hittest.js]

[browser_test_shadowroot.js]

[browser_test_text.js]

[browser_test_zoom.js]

accessible/tests/browser/hittest/browser_test_scroll_hittest.js

0 → 100644
+105 −0
Original line number Diff line number Diff line 
/* 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";



/**

 * Verify that hit testing returns the proper accessible when one accessible

 * covers another accessible due to scroll clipping. See Bug 1819741.

 */

addAccessibleTask(

  `

<div id="container" style="height: 100%; position: absolute; flex-direction: column; display: flex;">

  <div id="title-bar" style="height: 500px; background-color: red;"></div>

  <div id="message-container" style="overflow-y: hidden; display: flex;">

    <div style="overflow-y: auto;" id="message-scrollable">

      <p style="white-space: pre-line;">

        Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec dictum luctus molestie. Nam in libero mi. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas.



        Praesent aliquet semper libero, eu ullamcorper tortor vestibulum ac. Sed non pharetra sem. Quisque sodales ipsum a ipsum condimentum porttitor. Integer luctus pellentesque ipsum, eu dignissim nunc fermentum in.



        Etiam blandit nisl vitae dolor molestie faucibus. In euismod, massa vitae commodo bibendum, urna augue pharetra nibh, et sagittis libero est in ligula. Mauris tincidunt risus ornare, rutrum augue in, blandit ligula. Aenean ultrices vel risus sit amet varius.



        Vivamus pretium ultricies nisi a cursus. Integer cursus quam a metus ultricies, vel pulvinar nunc varius. Quisque facilisis lorem eget ipsum vehicula, laoreet congue lorem viverra.



        Praesent dignissim, diam sed semper ultricies, diam ex laoreet justo, ac euismod massa metus pharetra nunc. Vestibulum sapien erat, consequat at eleifend id, suscipit sit amet mi.



        Curabitur sed mauris vitae justo rutrum convallis ac sed justo. Ut nec est sed nisi feugiat egestas. Mauris accumsan mi eget nibh fermentum, in dignissim odio feugiat.



        Maecenas augue dolor, gravida ut ultrices ultricies, condimentum et dui. In sed augue fermentum, posuere velit et, pulvinar tellus. Morbi id fermentum quam, at varius arcu.



        Duis elementum vitae sapien id tincidunt. Aliquam velit ligula, sollicitudin eget placerat non, aliquam at erat. Pellentesque non porta metus. Mauris vel finibus sem, nec ullamcorper leo.



        Nulla sit amet lorem vitae diam consectetur porttitor a cursus massa. Sed id ornare lorem. Sed placerat facilisis ipsum et ultricies. Sed eu semper enim, ut aliquet odio.



        Sed nulla ex, pharetra vel porttitor congue, dictum et purus. Suspendisse vel risus sit amet nulla volutpat ullamcorper. Morbi et ullamcorper est. Pellentesque eget porta risus. Nullam non felis elementum, auctor massa et, consectetur neque.



        Fusce sit amet arcu finibus, ornare sem sed, tempus nibh. Donec rutrum odio eget bibendum pulvinar. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus.



        Phasellus sed risus diam. Vivamus mollis, risus ac feugiat pellentesque, ligula tortor finibus libero, et venenatis turpis lectus et justo. Suspendisse euismod mi at lectus sagittis dignissim. Mauris a ornare enim.

      </p>

    </div>

  </div>

  <div id="footer-bar" style="height: 500px; background-color: blue;"></div>

</div>

  `,

  async function(browser, docAcc) {

    const container = findAccessibleChildByID(docAcc, "container");

    const scrollable = findAccessibleChildByID(docAcc, "message-scrollable");

    const titleBar = findAccessibleChildByID(docAcc, "title-bar");

    const footerBar = findAccessibleChildByID(docAcc, "footer-bar");

    const dpr = await getContentDPR(browser);

    const [, , , titleBarHeight] = Layout.getBounds(titleBar, dpr);

    const [, , , scrollableHeight] = Layout.getBounds(scrollable, dpr);



    // Verify that the child at this point is not the underlying paragraph.

    info(

      "Testing that the deepest child at this point is the overlaid section, not the paragraph beneath it."

    );

    await testChildAtPoint(

      dpr,

      1,

      titleBarHeight - 1,

      container,

      titleBar,

      titleBar

    );

    await testChildAtPoint(

      dpr,

      1,

      titleBarHeight + scrollableHeight + 1,

      container,

      footerBar,

      footerBar

    );



    await invokeContentTask(browser, [], () => {

      // Scroll the text down.

      let elem = content.document.getElementById("message-scrollable");

      elem.scrollTo(0, elem.scrollHeight);

    });

    await waitForContentPaint(browser);



    info(

      "Testing that the deepest child at this point is still the overlaid section, after scrolling the paragraph."

    );

    await testChildAtPoint(

      dpr,

      1,

      titleBarHeight - 1,

      container,

      titleBar,

      titleBar

    );

    await testChildAtPoint(

      dpr,

      1,

      titleBarHeight + scrollableHeight + 1,

      container,

      footerBar,

      footerBar

    );

  },

  { chrome: true, iframe: true, remoteIframe: true }

);