Bug 242540

Summary:

AX: Suspending AX during sync IPC caused by deleting characters in a textbox can cause issues for AX clients

Product:

WebKit

Reporter:

Tyler Wilcock <tyler_w>

Component:

Accessibility

Assignee:

Tyler Wilcock <tyler_w>

Status:

NEW ---

Severity:

Normal

CC:

andresg_22, cfleizach, megan_gardner, sam, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Description Tyler Wilcock 2022-07-08 15:21:39 PDT

Deleting a character in a textbox causes DeleteSelectionCommand::doApply(), in turn calling Editor::textWillBeDeletedInTextField(), in turn performing sync IPC. This can cause issues for AX clients handling the notifications also caused by the deletion, such as AXValueChanged or AXSelectedTextChanged.

Comment 1 Radar WebKit Bug Importer 2022-07-08 15:21:47 PDT

<rdar://problem/96700714>

Comment 2 Tyler Wilcock 2022-07-08 15:32:33 PDT

Created attachment 460770 [details]
Patch

Comment 3 chris fleizach 2022-07-08 15:43:21 PDT

Comment on attachment 460770 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=460770&action=review

> Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.cpp:145
> +    if (messageName.contains("ShouldPerformActionInFormTextField"_s))

should we make this more forward looking and add a method like

bool shouldMessageInformProcessSuspension(String messageName) {
   static HashMap map = { "ShouldPerformActionInFormTextField" }
  return !map.contains(messageName)
}

or something to that effect?

Comment 4 Tyler Wilcock 2022-07-08 18:12:20 PDT

Created attachment 460773 [details]
Patch

Comment 5 Tyler Wilcock 2022-07-08 18:22:46 PDT

CC Sam who wrote this code a long time ago in https://github.com/WebKit/WebKit/commit/4e2b681b26d3407c4213815abe365bcf7c3936c3 and Megan for her editing knowledge.

Comment 6 Tyler Wilcock 2022-07-08 18:37:06 PDT

Actually, maybe textDidChangeInTextField is not the right fit here...maybe we need to create a new method on InjectedBundlePageFormClient called textWillBeDeletedInTextField or something? Neither textDidChangeInTextField from my current patch nor shouldPerformActionInTextField in current `main` really capture the intent well IMO (and the latter causes issues for AX due to its usage of sync IPC)