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

@@ -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<AccEvent> nameChangeEvent =
               new AccEvent(nsIAccessibleEvent::EVENT_NAME_CHANGE, parent);
           pushed |= PushEvent(nameChangeEvent);

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<AccStateChangeEvent> 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 <label>. Don't
+      // append the subtree text. Return " " to prevent
+      // nsTextEquivUtils::AppendFromAccessible walking the subtree itself.
+      aName += ' ';
+      return flag;
     }
   }
-
-  return NS_OK;
+  // Unfortunately, GetNameFromSubtree doesn't separate the button text from the
+  // file name text. Compute the text ourselves.
+  uint32_t count = ChildCount();
+  for (uint32_t c = 0; c < count; ++c) {
+    TextLeafAccessible* leaf = LocalChildAt(c)->AsTextLeaf();
+    MOZ_ASSERT(leaf);
+    if (!aName.IsEmpty()) {
+      aName += ' ';
+    }
+    aName += leaf->Text();
+  }
+  return flag;
 }
 
-LocalAccessible* HTMLFileInputAccessible::CurrentItem() const {
-  // Allow aria-activedescendant to override.
-  if (LocalAccessible* item = HyperTextAccessible::CurrentItem()) {
-    return item;
-  }
+bool HTMLFileInputAccessible::HasPrimaryAction() const { return true; }
 
-  // The HTML file input itself gets DOM focus, not the button inside it.
-  // For a11y, we want the button to get focus.
-  LocalAccessible* button = LocalFirstChild();
-  if (!button) {
-    MOZ_ASSERT_UNREACHABLE("File input doesn't contain a button");
-    return nullptr;
+void HTMLFileInputAccessible::ActionNameAt(uint8_t aIndex, nsAString& aName) {
+  if (aIndex == 0) {
+    aName.AssignLiteral("press");
   }
-  MOZ_ASSERT(button->IsButton());
-  return button;
 }
 
+bool HTMLFileInputAccessible::IsWidget() const { return true; }
+
 ////////////////////////////////////////////////////////////////////////////////
 // HTMLSpinnerAccessible
 ////////////////////////////////////////////////////////////////////////////////

accessible/html/HTMLFormControlAccessible.h

@@ -60,7 +60,6 @@ class HTMLButtonAccessible : public HyperTextAccessible {
 
   // LocalAccessible
   virtual mozilla::a11y::role NativeRole() const override;
-  virtual uint64_t State() override;
   virtual uint64_t NativeState() const override;
 
   // ActionAccessible
@@ -135,8 +134,11 @@ class HTMLFileInputAccessible : public HyperTextAccessible {
 
   // LocalAccessible
   virtual mozilla::a11y::role NativeRole() const override;
-  virtual nsresult HandleAccEvent(AccEvent* aAccEvent) override;
-  virtual LocalAccessible* CurrentItem() const override;
+  virtual bool IsAcceptableChild(nsIContent* aEl) const override;
+  virtual ENameValueFlag Name(nsString& aName) const override;
+  virtual bool HasPrimaryAction() const override;
+  virtual void ActionNameAt(uint8_t aIndex, nsAString& aName) override;
+  virtual bool IsWidget() const override;
 };
 
 /**

accessible/tests/browser/e10s/browser.toml

@@ -70,6 +70,8 @@ skip-if = ["os == 'win'"] # Bug 1288839
 
 ["browser_events_textchange.js"]
 
+["browser_file_input.js"]
+
 ["browser_language.js"]
 
 ["browser_obj_group.js"]

accessible/tests/browser/e10s/browser_file_input.js

@@ -0,0 +1,77 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+/* import-globals-from ../../mochitest/name.js */
+loadScripts({ name: "name.js", dir: MOCHITESTS_DIR });
+
+addAccessibleTask(
+  `
+<input type="file" id="noName">
+<input type="file" id="ariaLabel" aria-label="ariaLabel">
+<label>wrappingLabel <input type="file" id="wrappingLabel"></label>
+<label for="labelFor">labelFor</label> <input type="file" id="labelFor">
+<input type="file" id="title" title="title">
+  `,
+  async function (browser, docAcc) {
+    const browseButton = "Browse…";
+    const noFileSuffix = `${browseButton} No file selected.`;
+    const noName = findAccessibleChildByID(docAcc, "noName");
+    testName(noName, noFileSuffix);
+    const ariaLabel = findAccessibleChildByID(docAcc, "ariaLabel");
+    testName(ariaLabel, `ariaLabel ${noFileSuffix}`);
+    const wrappingLabel = findAccessibleChildByID(docAcc, "wrappingLabel");
+    testName(wrappingLabel, `wrappingLabel ${noFileSuffix}`);
+    const labelFor = findAccessibleChildByID(docAcc, "labelFor");
+    testName(labelFor, `labelFor ${noFileSuffix}`);
+    const title = findAccessibleChildByID(docAcc, "title");
+    testName(title, noFileSuffix);
+    testDescr(title, "title");
+
+    // Test that the name of the button changes correctly when a file is chosen.
+    function chooseFile(id) {
+      return invokeContentTask(browser, [id], contentId => {
+        const MockFilePicker = content.SpecialPowers.MockFilePicker;
+        MockFilePicker.init(content);
+        MockFilePicker.useBlobFile();
+        MockFilePicker.returnValue = MockFilePicker.returnOK;
+        const input = content.document.getElementById(contentId);
+        const inputReceived = new Promise(resolve =>
+          input.addEventListener(
+            "input",
+            event => {
+              MockFilePicker.cleanup();
+              resolve(event.target.files[0].name);
+            },
+            { once: true }
+          )
+        );
+        input.click();
+        return inputReceived;
+      });
+    }
+
+    info("noName: Choosing file");
+    let nameChanged = waitForEvent(EVENT_NAME_CHANGE, "noName");
+    const fn = await chooseFile("noName");
+    // e.g. "Browse…helloworld.txt"
+    const withFileSuffix = `${browseButton} ${fn}`;
+    await nameChanged;
+    testName(noName, withFileSuffix);
+
+    info("ariaLabel: Choosing file");
+    nameChanged = waitForEvent(EVENT_NAME_CHANGE, "ariaLabel");
+    await chooseFile("ariaLabel");
+    await nameChanged;
+    testName(ariaLabel, `ariaLabel ${withFileSuffix}`);
+
+    info("wrappingLabel: Choosing file");
+    nameChanged = waitForEvent(EVENT_NAME_CHANGE, "wrappingLabel");
+    await chooseFile("wrappingLabel");
+    await nameChanged;
+    testName(wrappingLabel, `wrappingLabel ${withFileSuffix}`);
+  },
+  { topLevel: true, chrome: true }
+);

accessible/tests/mochitest/elm/test_HTMLSpec.html

@@ -707,10 +707,8 @@
       // HTML:input@type="file"
 
       obj = {
-        GROUPING: [
-          { role: ROLE_PUSHBUTTON },
-          { role: ROLE_LABEL },
-        ],
+        role: ROLE_PUSHBUTTON,
+        actions: "press",
       };
       testElm("input_file", obj);
 

accessible/tests/mochitest/events/test_focus_controls.html

@@ -38,7 +38,7 @@
       gQueue.push(new changeCurrentItem("checkbox"));
       gQueue.push(new changeCurrentItem("radio1"));
 
-      let fileBrowseButton = getAccessible("file").firstChild;
+      let fileBrowseButton = getAccessible("file");
       gQueue.push(new synthFocus("file", new focusChecker(fileBrowseButton)));
 
       gQueue.invoke(); // Will call SimpleTest.finish();

accessible/tests/mochitest/events/test_statechange.html

@@ -61,11 +61,10 @@
     async function stateChangeOnFileInput(aID, aAttr, aValue,
                                     aState, aIsExtraState, aIsEnabled) {
       let fileControlNode = getNode(aID);
-      let fileControl = getAccessible(fileControlNode);
-      let browseButton = fileControl.firstChild;
-      let p = waitForEvents([
-        stateChangeEventArgs(fileControl, aState, aIsEnabled, aIsExtraState),
-        stateChangeEventArgs(browseButton, aState, aIsEnabled, aIsExtraState)])
+      let browseButton = getAccessible(fileControlNode);
+      let p = waitForStateChange(
+        browseButton, aState, aIsEnabled, aIsExtraState
+      );
       fileControlNode.setAttribute(aAttr, aValue);
       await p;
     }

accessible/tests/mochitest/role/test_general.html

@@ -50,8 +50,8 @@
       testRole("head5", ROLE_HEADING);
       testRole("head6", ROLE_HEADING);
 
-      // Test that an html:input @type="file" is exposed as ROLE_GROUPING.
-      testRole("data", ROLE_GROUPING);
+      // Test that an html:input @type="file" is exposed as ROLE_PUSHBUTTON.
+      testRole("data", ROLE_PUSHBUTTON);
 
       // Test that input type="checkbox" and type="radio" are
       // exposed as such regardless of appearance style.

accessible/tests/mochitest/states/test_aria.html

@@ -190,10 +190,9 @@
       testStates("as_item2", 0, 0, 0, EXT_STATE_ACTIVE);
 
       // universal ARIA properties inherited from file input control
-      var fileBrowseButton = getAccessible("fileinput").firstChild;
+      var fileBrowseButton = getAccessible("fileinput");
       testStates(fileBrowseButton,
                  STATE_BUSY | STATE_UNAVAILABLE | STATE_REQUIRED | STATE_HASPOPUP | STATE_INVALID);
-      // No states on the label.
 
       // offscreen test
       testStates("aria_offscreen_textbox", STATE_OFFSCREEN);

accessible/tests/mochitest/states/test_inputs.html

@@ -65,10 +65,9 @@
     testStates("f_input_disabled", STATE_UNAVAILABLE);
 
     // //////////////////////////////////////////////////////////////////////////
-    // inherited from file control
-    var fileBrowseButton = getAccessible("file").firstChild;
+    // file control
+    var fileBrowseButton = getAccessible("file");
     testStates(fileBrowseButton, STATE_UNAVAILABLE | STATE_REQUIRED);
-    // No states on the label.
 
     // //////////////////////////////////////////////////////////////////////////
     // 'invalid' state

accessible/tests/mochitest/tree/test_filectrl.html

@@ -17,19 +17,10 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=483573
   <script type="application/javascript">
     function doTest() {
       var accTree = {
-        role: ROLE_GROUPING,
+        role: ROLE_PUSHBUTTON,
         children: [
-          {
-            role: ROLE_PUSHBUTTON,
-          },
-          {
-            role: ROLE_LABEL,
-            children: [
-              {
-                role: ROLE_TEXT_LEAF,
-              },
-            ],
-          },
+          { role: ROLE_TEXT_LEAF },
+          { role: ROLE_TEXT_LEAF },
         ],
       };
       testAccessibleTree("filectrl", accTree);