Bug 210081 - AX: VoiceOver can't activate combobox when textfield is inside it
Summary: AX: VoiceOver can't activate combobox when textfield is inside it
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-06 16:43 PDT by chris fleizach
Modified: 2020-04-07 16:43 PDT (History)
21 users (show)

See Also:


Attachments
patch (31.72 KB, patch)
2020-04-06 16:46 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
patch (31.72 KB, patch)
2020-04-06 16:55 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
patch (35.45 KB, patch)
2020-04-07 11:56 PDT, chris fleizach
jdiggs: review+
Details | Formatted Diff | Diff
patch (35.92 KB, patch)
2020-04-07 14:59 PDT, chris fleizach
no flags Details | Formatted Diff | Diff

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