Bug 186690 - AX: setValue on text controls should send out key events
Summary: AX: setValue on text controls should send out key events
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-15 13:35 PDT by Nan Wang
Modified: 2018-07-06 10:30 PDT (History)
12 users (show)

See Also:


Attachments
patch (6.44 KB, patch)
2018-07-03 18:00 PDT, Nan Wang
cfleizach: review+
Details | Formatted Diff | Diff
patch (4.09 KB, patch)
2018-07-05 18:27 PDT, Nan Wang
no flags Details | Formatted Diff | Diff
patch (4.09 KB, patch)
2018-07-05 18:32 PDT, Nan Wang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nan Wang 2018-06-15 13:35:41 PDT
Currently for text controls we are just setting their value directly to the input elements. We should mimic the keyboard typing process and send out key events.
Comment 1 Radar WebKit Bug Importer 2018-06-15 13:35:53 PDT
<rdar://problem/41169985>
Comment 2 Nan Wang 2018-07-03 18:00:15 PDT
Created attachment 344249 [details]
patch

I think it would be more appropriate to dispatch InputEvent since we don't want to send out KeyboardEvent for each of the character in the value string.
Comment 3 chris fleizach 2018-07-04 12:58:36 PDT
Comment on attachment 344249 [details]
patch

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

> Source/WebCore/ChangeLog:9
> +        Use Editor's insertText function when the text control element is being

this should be just one sentence. 

"Use the Editor's insertText function when the text control element is being, so that the InputEvent will be dispatched properly."

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1784
> +    }

should we make the following statement an else if so this follows the rest of the pattern and we can ditch the return;
Comment 4 Nan Wang 2018-07-05 10:08:12 PDT
Comment on attachment 344249 [details]
patch

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

>> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1784
>> +    }
> 
> should we make the following statement an else if so this follows the rest of the pattern and we can ditch the return;

This if checks the frame. But we still want to follow the rest of the pattern if the element is not in editing mode. So I think a return here is more appropriate.
Comment 5 Nan Wang 2018-07-05 10:20:26 PDT
Committed r233525: <https://trac.webkit.org/changeset/233525>
Comment 6 Truitt Savell 2018-07-05 13:10:27 PDT
It looks like the new test:
accessibility/Mac/set-value-editable-dispatch-events.html

is flakey from the start. I have seen it fail on a few EWS patches. 

test History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=accessibility%2Fmac%2Fset-value-editable-dispatch-events.html
Comment 7 Nan Wang 2018-07-05 13:14:52 PDT
Ok I will take a look
Comment 8 Nan Wang 2018-07-05 18:27:24 PDT
Created attachment 344390 [details]
patch

patch to fix the flaky test.

I think the problem is that both clearText and insertText will dispatch input events. The event handling in the test can get messy in some of the machines. We should make sure handling all the input events before moving to the next test case.
Comment 9 Nan Wang 2018-07-05 18:30:34 PDT
Reopen to fix the flaky test
Comment 10 Nan Wang 2018-07-05 18:32:55 PDT
Created attachment 344391 [details]
patch
Comment 11 WebKit Commit Bot 2018-07-06 10:30:02 PDT
Comment on attachment 344391 [details]
patch

Clearing flags on attachment: 344391

Committed r233580: <https://trac.webkit.org/changeset/233580>
Comment 12 WebKit Commit Bot 2018-07-06 10:30:04 PDT
All reviewed patches have been landed.  Closing bug.