1825611 - [CTW] Hit testing overflow:hidden; nodes is incorrect

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Morgan Reschenberg [:morgan]]

STR:

1. Load the data:text/html,<style> div div { overflow: hidden; font-family: monospace; width: 2ch; } </style><div style="display: flex; flex-direction: row-reverse;"><div>abcde</div><div>fghij</div> with CtW on
2. Hit test "fg"

Expected: We receive "fg" as a hittest result
Actual: Hit testing reports the entire acc, including the hidden portions (fghij)

3. Hit test "ab"

Expected: we get "ab"
Actual: we get the "fg" node

Comment 1

[James Teh [:Jamie]]

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

Actual: Hit testing reports the entire acc, including the hidden portions (fghij)

I don't quite get this bit. Hit testing for the acc (ChildAtPoint) reports the "fghij"text Accessible, which makes sense; we can't get any smaller than that. The client then has to call OffsetAtPoint with the point in question to determine which character it's interested in.

However, hit testing h, i or j with ChildAtPoint shouldn't report the fg Accessible.

Comment 2

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1825611: Cache overflow r?Jamie

Comment 3

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1825611: Trim bounds when hittesting overflow:hidden containers r?Jamie

Depends on D175193

Comment 4

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1825611: Ensure nodes with overflow:hidden styling always create an accessible r?Jamie

Depends on D175194

Comment 5

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1825611: Add test for overflow:hidden hittesting and acc creation r?Jamie

Depends on D175195

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db2d550a788b
Cache overflow r=Jamie
https://hg.mozilla.org/integration/autoland/rev/c79ad1ee1e32
Trim bounds when hittesting overflow:hidden containers r=Jamie
https://hg.mozilla.org/integration/autoland/rev/ee5e3b614f91
Ensure nodes with overflow:hidden styling always create an accessible r=Jamie
https://hg.mozilla.org/integration/autoland/rev/a30a125f2aea
Add test for overflow:hidden hittesting and acc creation r=Jamie

Comment 7

[Iulian Moraru]

Backed out for causing multiple mochitest failures.

• Push with failures

• Failure log for ba jobs

• Failure log for a11y jobs

• Backout link

Comment 8

[Morgan Reschenberg [:morgan]]

https://treeherder.mozilla.org/jobs?repo=try&revision=fe498bfa3970e7475e2fa878047a0375137b3521

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e74c388d1a4e
Cache overflow r=Jamie
https://hg.mozilla.org/integration/autoland/rev/9152cb7808c2
Trim bounds when hittesting overflow:hidden containers r=Jamie
https://hg.mozilla.org/integration/autoland/rev/c2d6231e9361
Ensure nodes with overflow:hidden styling always create an accessible r=Jamie
https://hg.mozilla.org/integration/autoland/rev/169c3f65e1ce
Add test for overflow:hidden hittesting and acc creation r=Jamie

Comment 10

[Serban Stanca [:SerbanS]]

Backed out for causing mochitests failures in accessible/tests.

• Backout link
• Push with failures

---

• Failure Log - ba
• Failure line: TEST-UNEXPECTED-FAIL | accessible/tests/browser/tree/browser_general.js | Test timed out -

---

• Failure Log - a11y
• Failure line: TEST-UNEXPECTED-FAIL | accessible/tests/mochitest/actions/test_media.html | Scenario #0 of test with ID = 'name change handling' failed. 'name changed event is missed.

Comment 11

[Morgan Reschenberg [:morgan]]

https://treeherder.mozilla.org/jobs?repo=try&revision=d788d50e0f042959444b7e97f5dbcd416c31cbdc

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/920bae37ff3e
Cache overflow r=Jamie
https://hg.mozilla.org/integration/autoland/rev/7e2ffdd02a98
Trim bounds when hittesting overflow:hidden containers r=Jamie
https://hg.mozilla.org/integration/autoland/rev/683910281b5d
Ensure nodes with overflow:hidden styling always create an accessible r=Jamie
https://hg.mozilla.org/integration/autoland/rev/74462e3c7265
Add test for overflow:hidden hittesting and acc creation r=Jamie

Comment 13

[Natalia Csoregi [:nataliaCs]]

Backed out with Bug 1828373 and Bug 1825411 for accessible failures on on /browser_general.js

• backout: https://hg.mozilla.org/integration/autoland/rev/8d8d99e7581851de698e27339dffd60ee8dc8e32
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=fa9772a45927f265c2eb2fa7301061c3406441f9
• failure log: https://treeherder.mozilla.org/logviewer?job_id=414559862&repo=autoland&lineNumber=4569

[task 2023-05-03T23:03:41.235Z] 23:03:41     INFO - TEST-PASS | accessible/tests/browser/tree/browser_general.js | Wrong last child of [role: text leaf, name: 'hello world', address: [xpconnect wrapped nsIAccessible]] - 
[task 2023-05-03T23:03:41.236Z] 23:03:41     INFO - Adding overflow:hidden styling to div
[task 2023-05-03T23:03:41.236Z] 23:03:41     INFO - Buffered messages finished
[task 2023-05-03T23:03:41.237Z] 23:03:41     INFO - TEST-UNEXPECTED-FAIL | accessible/tests/browser/tree/browser_general.js | Test timed out - 
[task 2023-05-03T23:03:41.455Z] 23:03:41     INFO - GECKO(5468) | MEMORY STAT | vsize 2104178MB | vsizeMaxContiguous 66514862MB | residentFast 236MB | heapAllocated 91MB
[task 2023-05-03T23:03:41.459Z] 23:03:41     INFO - TEST-OK | accessible/tests/browser/tree/browser_general.js | took 46426ms

Comment 14

[Norisz Fay [:noriszfay]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/8d8d99e75818

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69b67a4f7465
Cache overflow r=Jamie
https://hg.mozilla.org/integration/autoland/rev/f008e16e684c
Trim bounds when hittesting overflow:hidden containers r=Jamie
https://hg.mozilla.org/integration/autoland/rev/a819a38858ad
Ensure nodes with overflow:hidden styling always create an accessible r=Jamie
https://hg.mozilla.org/integration/autoland/rev/29d02d013170
Add test for overflow:hidden hittesting and acc creation r=Jamie

Comment 16

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/69b67a4f7465
https://hg.mozilla.org/mozilla-central/rev/f008e16e684c
https://hg.mozilla.org/mozilla-central/rev/a819a38858ad
https://hg.mozilla.org/mozilla-central/rev/29d02d013170

Comment 17

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

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

• If yes, please nominate the patch for beta approval.Also, don't forget to request an uplift for the patches in the regressions caused by this fix.
• If no, please set status-firefox114 to wontfix.

For more information, please visit BugBot documentation.

Comment 18

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

backed out in 115 beta (115.0rc1) for causing bug 1840200 (backout patch is in bug 1840200)

Comment 19

[Monica Chiorean]

I was able to reproduce the issue on Mac 12.6 using FF build 115.0, using below steps:

1. Enable VoiceOver, In VoiceOver Utility.app > Navigation ensure "Synchronise VO focus and mouse cursor" is checked
2. Load the provided test case
3. Mouseover "fg" with VoiceOver enabled
   Expected:
   VO reports "fg" only
   Actual:
   VO reports all on-screen text.

Verified that issue is fixed on Mac 12.6 using FF build 116.0a1(20230627214548).

Comment 20

[James Teh [:Jamie]]

Comment on attachment 9328059 [details]
Bug 1825611: Cache overflow r?Jamie

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed to fix various accessibility regressions introduced by the Cache the World project which shipped in 115.
• User impact if declined: Screen reader mouse and touch tracking problems on popular sites such as LinkedIn.
• Fix Landed on Version: 116
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): These patches did cause regressions. However, these have since been addressed and those fixes have been baking on release since at least 117.

Comment 21

[James Teh [:Jamie]]

For the record, I'm not requesting the uplift of bug 1832449, which is a crash introduced by these patches. However, the fix for that bug is incorporated into the patches I'm requesting an uplift for here. The reason is that the backout in bug 1840200 didn't actually remove some of the checks added by bug 1832449, so I had to manually tweak the patches in this bug to compensate.

Comment 22

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/71f67b3d91f4
https://hg.mozilla.org/releases/mozilla-esr115/rev/01086a445655
https://hg.mozilla.org/releases/mozilla-esr115/rev/fa87c3742028
https://hg.mozilla.org/releases/mozilla-esr115/rev/5cd8992c2ae8

Comment 23

[Donal Meehan [:dmeehan]]

Comment on attachment 9328059 [details]
Bug 1825611: Cache overflow r?Jamie

Approved for 115.4esr.

Comment 24

[Donal Meehan [:dmeehan]]

Comment on attachment 9328060 [details]
Bug 1825611: Trim bounds when hittesting overflow:hidden containers r?Jamie

Approved for 115.4esr.

Comment 25

[Donal Meehan [:dmeehan]]

Comment on attachment 9328061 [details]
Bug 1825611: Ensure nodes with overflow:hidden styling always create an accessible r?Jamie

Approved for 115.4esr.

Comment 26

[Donal Meehan [:dmeehan]]

Comment on attachment 9328062 [details]
Bug 1825611: Add test for overflow:hidden hittesting and acc creation r?Jamie

Approved for 115.4esr.