1819741 - [CTW] Hit testing reports Accessibles hidden by another element with overflow: auto/scroll

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect, P2)

Description

[Nathan LaPré]

Attachment: overflow_hidden_reproducer.html

STR:

0. Start NVDA and ensure the accessibility cache is enabled.
1. Load the file attached to this bug.
2. Focus the title-bar element. If you're using a mouse, move it around the title bar area. Note that NVDA only reads the title-bar contents.
3. Scroll the message-scrollable element down to the bottom. The top part of the message will be hidden by the title bar as it scrolls.
4. Focus the title-bar element again and move the mouse around the title bar area.

Expected: NVDA will not read the message text hidden behind the title bar.
Actual: NVDA reads the message text hidden behind the title bar.

Note that the problem doesn't exist if the title bar element comes after the message in the DOM.

It seems that this has to do with us considering the bounds of the scrolled accessible to extend "underneath" the title bar, which is not the way that layout thinks of it. For the purposes of hit testing, we should probably pretend that the bounds of the scrolled accessible are just what's visible on screen.

Comment 1

[James Teh [:Jamie]]

I think the key here is overflow: auto/scroll, rather than overflow: hidden. With hidden, you can't scroll, so there's nothing for scrolling to clip out. I realise the parent has overflow: hidden, but I think the issue occurs with the child having overflow: auto.

Does this make sense or is there a reason you think it's the hidden part?

Comment 2

[Nathan LaPré]

You're totally right, I'll update the wording on this bug title.

Comment 3

[Morgan Reschenberg [:morgan]]

Jamie and I discussed this in our 1:1 -- here's some notes !

First, it is important to note that this issue only occurs for elements that are partially onscreen -- if you add a paragraph above the one in your example (wrapping both in a div so they're one flex layout object) and scroll the first paragraph completely offscreen, it won't be hittestable. The second one, with content partially on screen will still hittest in its entirety. Accs with on-screen children are contained in the viewport cache even if they themselves are partially (or completely) obscured. When the paragraph in your example is scrolled behind the title bar the obscured text within it is considered offscreen but the paragraph itself is not because it has onscreen children (the text still within the visible scrollframe region). The fact that the paragraph exists in the viewport cache is technically correct, but we want to manipulate the bounds of partially on screen content such that they are bounded/masked by the scroll area.

How can we do that? There are probably multiple ways :) but the one I proposed to Jamie was:

• Add a aBoundsAreForHittesting arg to BoundsWithOffset, and when true:
• When we encounter a scroll area while computing acc bounds in BoundsWithOffset, subtract the scroll offset as we normally do
• Then, compute the intersection of the scroll-relative bounds rect and the scroll area itself. Negative Y coords should be clamped to 0 and Y coords larger than the scroll area's height should be set to that height -- we should do something similar in the X/width case. We'll also need to adjust the width/height of the relative bounds rect appropriately (we aren't just moving the rect's origin)
• We should do this before adding in the scroll area's coords -- we want to work with the coords of the scrolled content relative to the scroll area, not the scroll area itself.
• Change the BoundsWithOffset call in ChildAtPoint to have aBoudnsAreForHittesting = true

Does that make sense?

I'm not 100% sure if we want to do this only when hittesting, or if it'd be better to do this for bounds always.
Off the top of my head, it seems fine to do this always -- since that'd put us closer to matching bounds against what gets visibly rendered.
:Jamie do you have thoughts on that?

Comment 4

[James Teh [:Jamie]]

I'm not 100% sure if we want to do this only when hittesting, or if it'd be better to do this for bounds always.

I think we should only do this for hit testing.

that'd put us closer to matching bounds against what gets visibly rendered.

That's true, but it's also inconsistent with how bounds have been reported to clients for 20+ years. I don't think we should mess with that. :)

Comment 5

[Nathan LaPré]

(In reply to Morgan Reschenberg [:morgan] from comment #3)

Does that make sense?

Yeah, thanks for the explanation! I think I'm mostly with you, but I have a few questions after poking at this a bit:

• By "scroll-relative bounds rect," do you mean the variable remoteBounds variable after ApplyScrollOffset (i.e., after it's been made scroll-relative)?
• By "scroll area," do you mean the variable bounds as calculated up to this point? Or something else?
• We want to compute that intersection rect, but what should we do with it? Should we apply it to the existing bounds rect?

I've tried a few things without success as of yet, so I want to be sure I have this down conceptually. Thank you :)

Comment 6

[Morgan Reschenberg [:morgan]]

(In reply to Nathan LaPré from comment #5)

(In reply to Morgan Reschenberg [:morgan] from comment #3)

Does that make sense?

Yeah, thanks for the explanation! I think I'm mostly with you, but I have a few questions after poking at this a bit:

• By "scroll-relative bounds rect," do you mean the variable remoteBounds variable after ApplyScrollOffset (i.e., after it's been made scroll-relative)?

Yep yep you got it

• By "scroll area," do you mean the variable bounds as calculated up to this point? Or something else?

I use scroll area/scroll frame/accessible that holds scrollable content interchangeably. To determine if an acc represents a scroll area, we can check if it has a cached scroll position.

• We want to compute that intersection rect, but what should we do with it? Should we apply it to the existing bounds rect?

Yep. The final bounds rect we return should represent the intersection of this with its ancestor scroll area(s) in screen coordinates

Comment 7

[James Teh [:Jamie]]

I'm re-triaging to s3 because this could be pretty nasty for a low vision screen reader user who gets caught in this situation, since their AT could be reading content that isn't actually visible to them. We haven't had a user report, but the fact that this happens on LinkedIn is enough for me to think it will probably be encountered by a user eventually.

Comment 8

[Nathan LaPré]

Attachment: 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.

Comment 9

[Nathan LaPré]

Attachment: Bug 1819741: Part 2: Apply scroll offset before transform, r?morgan

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

Depends on D173392

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89ca086af3ad
Constrain acc bounds to that of scroll areas for hit testing, r=morgan

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/89ca086af3ad

Comment 12

[Anca Soncutean, Desktop QA]

Reproduced on Fx 112.0a1 (2023-03-01) with Windows 10.
Verified fixed with Fx 114.0a1 (2023-05-04) and Fx 113.0 on Windows 10.