1774705 - [CTW] Changing CSS position properties doesn't update cached bounds

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie] (away returning 28 Jan)]

STR:

1. Open this test case:
   data:text/html,<div id="container" style="position: absolute; top: 100px; left: 0px;">Hello</div><button onclick="container.style.left = '600px';">Move</button>
2. Get the bounds of the div Accessible.
3. Press the Move button.
4. Get the bounds of the div Accessible.
   • Expected: The div Accessible should have a larger left coordinate than in step 2.
   • Actual: The left coordinate remains the same as in step 2.

Perhaps this doesn't cause reflow, so we don't get notified the bounds might have changed, though this needs to be verified.

This also applies to animating position properties:

data:text/html,<style>.container { position: absolute; left: 0px; top: 100px; } .animate { left: 600px; transition: left 5s linear; }</style><div id="container" class="container">Hello</div><button onclick="container.classList.add('animate');">Animate</button>

Interestingly, animating the position using a transform does update cached bounds correctly because of the way we monitor for transform changes:
data:text/html,<style>.container { position: absolute; left: 0px; top: 100px; } .animate { transform: translateX(600px); transition: transform 5s linear; }</style><div id="container" class="container">Hello</div><button onclick="container.classList.add('animate');">Animate</button>

Perhaps we need to monitor the left, top, width and height CSS properties in a similar way to transforms for positioned elements; i.e. when position is not static.

Comment 1

[James Teh [:Jamie] (away returning 28 Jan)]

Attachment: Bug 1774705 part 2: Queue a bounds cache update when the CSS left/right/top/bottom CSS properties change on positioned elements.

Comment 2

[James Teh [:Jamie] (away returning 28 Jan)]

Attachment: Bug 1774705 part 1: Fall back to `this` when hit testing a cached RemoteAccessible if the point is inside `this`.

Previously, if the requested point was inside a different subtree to the origin Accessible (e.g. out-of-flow), we would return null because the origin wasn't an ancestor of the target.
We still check the ancestry, but if the point is inside the origin, we return the origin.
This is consistent with LocalAccessible (and thus non-cached RemoteAccessible).
This change is necessary here because our test for out-of-flow hit testing changes the CSS left property to move an out-of-flow element.
That test passed because the bounds cache for that element was never updated, so we always found the target we expected.
In the next patch, we fix the bounds cache update, which would break this test without this hit testing fix.
In addition, this patch also changes our test for overlapping elements to match this new expectation.
Since this is now consistent, we can remove the cache restriction and test LocalAccessible as well.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2febc1d0631d
part 1: Fall back to `this` when hit testing a cached RemoteAccessible if the point is inside `this`. r=morgan
https://hg.mozilla.org/integration/autoland/rev/2348189bd580
part 2: Queue a bounds cache update when the CSS left/right/top/bottom CSS properties change on positioned elements. r=morgan

Comment 4

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/2febc1d0631d
https://hg.mozilla.org/mozilla-central/rev/2348189bd580

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:Jamie, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox110 to wontfix.

For more information, please visit auto_nag documentation.

Comment 6

[James Teh [:Jamie] (away returning 28 Jan)]

Comment on attachment 9312287 [details]
Bug 1774705 part 1: Fall back to this when hit testing a cached RemoteAccessible if the point is inside this.

Beta/Release Uplift Approval Request

• User impact if declined: Accessibility screen coordinates/hit testing will be inaccurate in some cases on Windows/Linux beta.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only affects a specific area of the accessibility cache, which is being tested on beta but is not yet enabled on release. Covered by automated tests.
• String changes made/needed:
• Is Android affected?: No

Comment 7

[Pascal Chevrel:pascalc]

Comment on attachment 9312287 [details]
Bug 1774705 part 1: Fall back to this when hit testing a cached RemoteAccessible if the point is inside this.

Approved for 110 beta 5, thanks.

Comment 8

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/46b6c39d84a0
https://hg.mozilla.org/releases/mozilla-beta/rev/0cad81aa7b23

Comment 9

[Negritas Sergiu, Desktop QA]

Hello! I have managed to reproduce the issue with firefox 103.0a1(2022-06-15) on Ubuntu 22.04, I can confirm that the issue is fixed on firefox 111.0a1(2023-01-26) and 110.0b5 on Ubuntu 22.04, MacOS 11 and Windows 10.

I will update the status of the bug, the corresponding flags .

Have a nice day!