Bug 242540 - AX: Suspending AX during sync IPC caused by deleting characters in a textbox can cause issues for AX clients
Summary: AX: Suspending AX during sync IPC caused by deleting characters in a textbox ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tyler Wilcock
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-07-08 15:21 PDT by Tyler Wilcock
Modified: 2022-07-11 09:08 PDT (History)
6 users (show)

See Also:

Attachments
Patch (2.81 KB, patch)
2022-07-08 15:32 PDT, Tyler Wilcock 		no flags Details | Formatted Diff | Diff
Patch (2.68 KB, patch)
2022-07-08 18:12 PDT, Tyler Wilcock 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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)