Bug 210081

Summary: AX: VoiceOver can't activate combobox when textfield is inside it
Product: WebKit Reporter: chris fleizach <cfleizach>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cdumez, cmarcelo, dino, dmazzoni, esprehn+autocc, ews-watchlist, fmalita, gyuyoung.kim, jcraig, jdiggs, kangil.han, mifenton, pdr, sabouhallawa, samuel_white, schenney, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
none
patch
none
patch
jdiggs: review+
patch none

Description chris fleizach 2020-04-06 16:43:01 PDT
1.Enable VoiceOver 
2. Navigate to https://help.doordash.com/consumers/s/contactsupport?language=en_US
3. Attempt to activate the “Subcategory” dropdown menu box

RESULTS:
User is unable to activate box and interact with values within it.

<rdar://problem/60888469>
Comment 1 chris fleizach 2020-04-06 16:46:14 PDT
Created attachment 395632 [details]
patch
Comment 2 chris fleizach 2020-04-06 16:55:05 PDT
Created attachment 395633 [details]
patch
Comment 3 Andres Gonzalez 2020-04-07 07:29:32 PDT
(In reply to chris fleizach from comment #2)
> Created attachment 395633 [details]
> patch

--- a/LayoutTests/accessibility/activation-of-input-field-inside-other-element.html
+++ a/LayoutTests/accessibility/activation-of-input-field-inside-other-element.html
+<div role="combobox" aria-expanded="false" aria-haspopup="listbox" id="combo" onclick="comboBoxClicked();">
+   <div role="none">
+       <input id="input-40" type="text" role="textbox" autocomplete="off" placeholder="Select an Option" aria-autocomplete="none" readonly="" disabled="">
 Indent is short one char.

+<input id="element1" type="image" src="url-test1.html"><BR>
+<a id="element2" href="#url-test2.html">test</a><BR>
+<img id="element3" src="url-test3.png" width=100 height=100 alt="test"><BR>

Do we need these? Not used.

--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
+++ a/Source/WebCore/accessibility/AccessibilityObject.cpp
+    if (!dispatchedEvent)
+        dispatchedEvent = pressElement->accessKeyAction(true);
+
+    if (!dispatchedEvent)
+        pressElement->dispatchSimulatedClick(nullptr, SendMouseUpDownEvents);
+
     return true;

Instead of above, it would be cleaner to do:
    return dispatchedEvent || pressElement->accessKeyAction(true) || pressElement->dispatchSimulatedClick(nullptr, SendMouseUpDownEvents);

but dispatchSimulatedClick will have to return bool.

--- a/Source/WebCore/html/BaseCheckableInputType.cpp
+++ a/Source/WebCore/html/BaseCheckableInputType.cpp
+    bool dispatched = InputType::accessKeyAction(sendMouseEvents);

     ASSERT(element());
-    element()->dispatchSimulatedClick(0, sendMouseEvents ? SendMouseUpDownEvents : SendNoEvents);
+    if (!dispatched)
+        element()->dispatchSimulatedClick(0, sendMouseEvents ? SendMouseUpDownEvents : SendNoEvents);
+    return true;

Likewise, this could be:

    ASSERT(element());
    return InputType::accessKeyAction(sendMouseEvents) || element()->dispatchSimulatedClick(0, sendMouseEvents ? SendMouseUpDownEvents : SendNoEvents);

--- a/Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp
+++ a/Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp
     BaseDateAndTimeInputType::accessKeyAction(sendMouseEvents);
     ASSERT(element());
-    BaseClickableWithKeyInputType::accessKeyAction(*element(), sendMouseEvents);
+    return BaseClickableWithKeyInputType::accessKeyAction(*element(), sendMouseEvents);

Double click for date and time input?

--- a/Source/WebCore/html/HTMLOptionElement.cpp
+++ a/Source/WebCore/html/HTMLOptionElement.cpp
+bool HTMLOptionElement::accessKeyAction(bool)
 {
     RefPtr<HTMLSelectElement> select = ownerSelectElement();
     if (select)
         select->accessKeySetSelectedIndex(index());
+    return false;
 }

Shouldn't be:

     if (select)
         return select->accessKeySetSelectedIndex(index());

or at least:
     if (select) {
         select->accessKeySetSelectedIndex(index());
         return true;
    }

--- a/Source/WebCore/html/RangeInputType.cpp
+++ a/Source/WebCore/html/RangeInputType.cpp
+bool RangeInputType::accessKeyAction(bool sendMouseEvents)
 {
-    InputType::accessKeyAction(sendMouseEvents);
-
-    if (auto* element = this->element())
+    bool dispatchedClick = InputType::accessKeyAction(sendMouseEvents);
+    if (dispatchedClick)
+        return dispatchedClick;
+
+    if (auto* element = this->element()) {
         element->dispatchSimulatedClick(0, sendMouseEvents ? SendMouseUpDownEvents : SendNoEvents);
+        return true;
+    }
+
+    return false;
 }

Again this would benefit from dispatchSimulatedClick returning bool:
{
    auto* element = this->element();
    return InputType::accessKeyAction(sendMouseEvents) || (element && element->dispatchSimulatedClick(0, sendMouseEvents ? SendMouseUpDownEvents : SendNoEvents));
}

Do we need double click in this type?
Comment 4 chris fleizach 2020-04-07 11:56:28 PDT
Created attachment 395718 [details]
patch

Updated patch to address comments. Made dispatchSimulatedClick return a bool
Comment 5 chris fleizach 2020-04-07 14:59:30 PDT
Created attachment 395742 [details]
patch
Comment 6 EWS 2020-04-07 16:43:35 PDT
Committed r259687: <https://trac.webkit.org/changeset/259687>

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