From 53b2e98767167b6df405ed2947c01fba6efbaa13 Mon Sep 17 00:00:00 2001 From: James Teh Date: Mon, 23 Oct 2023 23:38:13 +0000 Subject: [PATCH] Bug 1661923: Expose file input as a button in the a11y tree rather than a group. r=eeejay A file input contains two native anonymous children: the Browse button and the file name label. Previously, we exposed the file input as a group in the a11y tree and its anonymous children as children of that group. While this is semantically correct, it causes several problems for screen readers. First, if the author provides a label or description, that gets exposed on the group. Some screen readers ignore either one or the other depending on the screen reader, what the author specified and how the user navigated there. Second, the file name label isn't focusable and wasn't associated to the group in any way aside from being a child. This meant that a screen reader user might not perceive it in some cases. Since most users understand a file input as a single control anyway, we now just expose the input as a simple button containing two text leaves. However, unlike most buttons, we need to append the text to the name even if the author specifies a name. As a bonus, this simplifies some code, since we no longer need to redirect focus or events. An additional problem was that the file input previously returned false for LocalAccessible::IsWidget, which meant that a wrapping HTML label wasn't associated correctly. This has been fixed as well, although this fix could have applied just as easily to the previous group implementation. Differential Revision: https://phabricator.services.mozilla.com/D191264 --- accessible/base/EventQueue.cpp | 4 +- accessible/html/HTMLFormControlAccessible.cpp | 98 +++++++++---------- accessible/html/HTMLFormControlAccessible.h | 8 +- accessible/tests/browser/e10s/browser.toml | 2 + .../tests/browser/e10s/browser_file_input.js | 77 +++++++++++++++ .../tests/mochitest/elm/test_HTMLSpec.html | 6 +- .../mochitest/events/test_focus_controls.html | 2 +- .../mochitest/events/test_statechange.html | 9 +- .../tests/mochitest/role/test_general.html | 4 +- .../tests/mochitest/states/test_aria.html | 3 +- .../tests/mochitest/states/test_inputs.html | 5 +- .../tests/mochitest/tree/test_filectrl.html | 15 +-- 12 files changed, 147 insertions(+), 86 deletions(-) create mode 100644 accessible/tests/browser/e10s/browser_file_input.js diff --git a/accessible/base/EventQueue.cpp b/accessible/base/EventQueue.cpp index 20d9905117b4..b2b71ccebb63 100644 --- a/accessible/base/EventQueue.cpp +++ b/accessible/base/EventQueue.cpp @@ -83,7 +83,9 @@ bool EventQueue::PushNameOrDescriptionChange(AccEvent* aOrigEvent) { nsAutoString name; ENameValueFlag nameFlag = parent->Name(name); // If name is obtained from subtree, fire name change event. - if (nameFlag == eNameFromSubtree) { + // HTML file inputs always get part of their name from the subtree, even + // if the author provided a name. + if (nameFlag == eNameFromSubtree || parent->IsHTMLFileInput()) { RefPtr nameChangeEvent = new AccEvent(nsIAccessibleEvent::EVENT_NAME_CHANGE, parent); pushed |= PushEvent(nameChangeEvent); diff --git a/accessible/html/HTMLFormControlAccessible.cpp b/accessible/html/HTMLFormControlAccessible.cpp index 856b3632b603..a5baeefcbaf0 100644 --- a/accessible/html/HTMLFormControlAccessible.cpp +++ b/accessible/html/HTMLFormControlAccessible.cpp @@ -14,6 +14,7 @@ #include "Relation.h" #include "mozilla/a11y/Role.h" #include "States.h" +#include "TextLeafAccessible.h" #include "nsContentList.h" #include "mozilla/dom/HTMLInputElement.h" @@ -174,22 +175,6 @@ void HTMLButtonAccessible::ActionNameAt(uint8_t aIndex, nsAString& aName) { if (aIndex == eAction_Click) aName.AssignLiteral("press"); } -uint64_t HTMLButtonAccessible::State() { - uint64_t state = HyperTextAccessible::State(); - if (state == states::DEFUNCT) return state; - - // Inherit states from input@type="file" suitable for the button. Note, - // no special processing for unavailable state since inheritance is supplied - // other code paths. - if (mParent && mParent->IsHTMLFileInput()) { - uint64_t parentState = mParent->State(); - state |= parentState & (states::BUSY | states::REQUIRED | states::HASPOPUP | - states::INVALID); - } - - return state; -} - uint64_t HTMLButtonAccessible::NativeState() const { uint64_t state = HyperTextAccessible::NativeState(); @@ -467,55 +452,62 @@ HTMLFileInputAccessible::HTMLFileInputAccessible(nsIContent* aContent, DocAccessible* aDoc) : HyperTextAccessible(aContent, aDoc) { mType = eHTMLFileInputType; + mGenericTypes |= eButton; } -role HTMLFileInputAccessible::NativeRole() const { - // No specific role in AT APIs. We use GROUPING so that the label will be - // reported by screen readers when focus enters this control . - return roles::GROUPING; +role HTMLFileInputAccessible::NativeRole() const { return roles::PUSHBUTTON; } + +bool HTMLFileInputAccessible::IsAcceptableChild(nsIContent* aEl) const { + // File inputs are rendered using native anonymous children. However, we + // want to expose this as a button Accessible so that clients can pick up the + // name and description from the button they activate, rather than a + // container. We still expose the text leaf descendants so we can get the + // name of the Browse button and the file name. + return aEl->IsText(); } -nsresult HTMLFileInputAccessible::HandleAccEvent(AccEvent* aEvent) { - nsresult rv = HyperTextAccessible::HandleAccEvent(aEvent); - NS_ENSURE_SUCCESS(rv, rv); - - // Redirect state change events for inherited states to child controls. Note, - // unavailable state is not redirected. That's a standard for unavailable - // state handling. - AccStateChangeEvent* event = downcast_accEvent(aEvent); - if (event && (event->GetState() == states::BUSY || - event->GetState() == states::REQUIRED || - event->GetState() == states::HASPOPUP || - event->GetState() == states::INVALID)) { - LocalAccessible* button = LocalChildAt(0); - if (button && button->Role() == roles::PUSHBUTTON) { - RefPtr childEvent = new AccStateChangeEvent( - button, event->GetState(), event->IsStateEnabled(), - event->FromUserInput()); - nsEventShell::FireEvent(childEvent); +ENameValueFlag HTMLFileInputAccessible::Name(nsString& aName) const { + ENameValueFlag flag = HyperTextAccessible::Name(aName); + if (flag == eNameFromSubtree) { + // The author didn't provide a name. We'll compute the name from our subtree + // below. + aName.Truncate(); + } else { + // The author provided a name. We do use that, but we also append our + // subtree text so the user knows this is a file chooser button and what + // file has been chosen. + if (aName.IsEmpty()) { + // Name computation is recursing, perhaps due to a wrapping