Closed Bug 1774705 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Disability Access APIs, defect)

defect

Tracking

()

VERIFIED FIXED
111 Branch
Tracking Status
firefox110 --- verified
firefox111 --- verified

People

(Reporter: Jamie, Assigned: Jamie)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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.

Assignee: nobody → jteh
Status: NEW → ASSIGNED

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.

Attachment #9312129 - Attachment description: Bug 1774705: Queue a bounds cache update when the CSS left/right/top/bottom CSS properties change on positioned elements. → Bug 1774705 part 2: Queue a bounds cache update when the CSS left/right/top/bottom CSS properties change on positioned elements.
Pushed by mreschenberg@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 111 Branch

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.

Flags: needinfo?(jteh)

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
Flags: needinfo?(jteh)
Attachment #9312287 - Flags: approval-mozilla-beta?
Attachment #9312129 - Flags: approval-mozilla-beta?

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.

Attachment #9312287 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9312129 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

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!

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: