Bug 252321

Summary: AX: In rare scenarios, WebKit fails to expose the text associated with various types of elements / nodes (text, headings)
Product: WebKit Reporter: Tyler Wilcock <tyler_w>
Component: AccessibilityAssignee: Tyler Wilcock <tyler_w>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Tyler Wilcock 2023-02-15 09:49:25 PST 
This results in accessibility clients not getting the information they need to make the correct announcement.
Comment 1 Radar WebKit Bug Importer 2023-02-15 09:49:36 PST 
<rdar://problem/105501379>
Comment 2 Tyler Wilcock 2023-02-15 20:23:21 PST 
rdar://102025959
Comment 3 Tyler Wilcock 2023-02-15 20:33:47 PST 
Created attachment 465018 [details]
Patch
Comment 4 chris fleizach 2023-02-16 10:24:46 PST 
Comment on attachment 465018 [details]
Patch

Talked with Tyler and given that the new test exercises this code path without causing problems is a good sign that this is correct behavior
Comment 5 Tyler Wilcock 2023-02-16 10:26:16 PST 
For more context, these checks were originally added in:

https://github.com/WebKit/WebKit/commit/e55881c553216d5c417ed9ac9dbb48c079e689ec

(search for "recalc")

That commit added a test called accessibility/heading-crash-after-hidden.html which we still pass in release and debug with this change.
Comment 6 Tyler Wilcock 2023-02-16 21:31:17 PST 
Created attachment 465043 [details]
Patch
Comment 7 Andres Gonzalez 2023-02-17 14:04:44 PST 
(In reply to Tyler Wilcock from comment #6)
> Created attachment 465043 [details]
> Patch

diff --git a/LayoutTests/accessibility/empty-text-under-element-cached.html b/LayoutTests/accessibility/empty-text-under-element-cached.html
new file mode 100644
index 000000000000..d74525ca8d60
--- /dev/null
+++ b/LayoutTests/accessibility/empty-text-under-element-cached.html

+    finishJSTest();

Do you need this? I didn't see jsTestIsAsync = true;

This covers the test case where the iso tree is built for the first time. Should we cover the case where a change happens after the iso tree is built and needs an update? Maybe adding a <l> and a <LI> with some text which was the case in QA's report.
Comment 8 Tyler Wilcock 2023-02-17 20:06:10 PST 
Created attachment 465063 [details]
Patch
Comment 9 Tyler Wilcock 2023-02-18 11:16:16 PST 
Created attachment 465066 [details]
Patch
Comment 10 Tyler Wilcock 2023-02-18 11:18:40 PST 
(In reply to Andres Gonzalez from comment #7)
> This covers the test case where the iso tree is built for the first time.
> Should we cover the case where a change happens after the iso tree is built
> and needs an update? Maybe adding a <l> and a <LI> with some text which was
> the case in QA's report.
OK, added this additional testcase in the latest patch.
Comment 11 EWS 2023-02-18 16:44:36 PST 
Committed 260521@main (87adaeb6e078): <https://commits.webkit.org/260521@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 465066 [details].