From 349756fe0b329b34887007b4ee13fc2cc13ba808 Mon Sep 17 00:00:00 2001
From: James Teh <jteh@mozilla.com>
Date: Wed, 27 Jul 2022 21:35:17 +0000
Subject: [PATCH] Bug 1780713: Fix a11y exposure of checkable XUL
 buttons/toolbarbuttons. r=eeejay

1. Expose role TOGGLE_BUTTON without the CHECKABLE state.
2. Fire a state change event when the button is toggled.

Differential Revision: https://phabricator.services.mozilla.com/D152486
---
 accessible/tests/mochitest/events/a11y.ini    |  2 +-
 ...tabpanels.xhtml => test_statechange.xhtml} | 23 ++++++++++++++--
 .../mochitest/states/test_controls.xhtml      |  2 +-
 .../tests/mochitest/tree/test_button.xhtml    | 11 ++++++++
 accessible/xul/XULFormControlAccessible.cpp   | 27 ++++++++++++++-----
 accessible/xul/XULFormControlAccessible.h     |  1 +
 6 files changed, 55 insertions(+), 11 deletions(-)
 rename accessible/tests/mochitest/events/{test_statechange_tabpanels.xhtml => test_statechange.xhtml} (77%)

diff --git a/accessible/tests/mochitest/events/a11y.ini b/accessible/tests/mochitest/events/a11y.ini
index 9f25e20370106..efe9aa3ee6c62 100644
--- a/accessible/tests/mochitest/events/a11y.ini
+++ b/accessible/tests/mochitest/events/a11y.ini
@@ -58,7 +58,7 @@ skip-if = os == 'mac'
 skip-if = os == 'mac'
 [test_selection_aria.html]
 [test_statechange.html]
-[test_statechange_tabpanels.xhtml]
+[test_statechange.xhtml]
 [test_stylechange.html]
 [test_text.html]
 [test_text_alg.html]
diff --git a/accessible/tests/mochitest/events/test_statechange_tabpanels.xhtml b/accessible/tests/mochitest/events/test_statechange.xhtml
similarity index 77%
rename from accessible/tests/mochitest/events/test_statechange_tabpanels.xhtml
rename to accessible/tests/mochitest/events/test_statechange.xhtml
index 90e8fad75bab3..4d63c664f1317 100644
--- a/accessible/tests/mochitest/events/test_statechange_tabpanels.xhtml
+++ b/accessible/tests/mochitest/events/test_statechange.xhtml
@@ -4,7 +4,7 @@
                  type="text/css"?>
 
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
-        title="tabpanels state change event tests">
+        title="XUL state change event tests">
 
   <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
   <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"/>
@@ -32,7 +32,7 @@
       ];
     }
 
-    async function doTests() {
+    async function testTabpanels() {
       const tabs = getNode("tabs");
       is(tabs.selectedIndex, 0, "tab1 initially selected");
       const panel1 = getAccessible("panel1");
@@ -65,7 +65,24 @@
       info("Selecting tab1");
       tabs.selectedIndex = 0;
       await events;
+    }
+
+    async function testPressed() {
+      const toolbarbuttonCheckbox = getNode("toolbarbuttonCheckbox");
+      testStates(toolbarbuttonCheckbox, 0, 0, STATE_PRESSED);
+      info("Checking toolbarbuttonCheckbox");
+      let changed = waitForStateChange(toolbarbuttonCheckbox, STATE_PRESSED, true);
+      toolbarbuttonCheckbox.setAttribute("checked", true);
+      await changed;
+      info("Unchecking toolbarbuttonCheckbox");
+      changed = waitForStateChange(toolbarbuttonCheckbox, STATE_PRESSED, false);
+      toolbarbuttonCheckbox.removeAttribute("checked");
+      await changed;
+    }
 
+    async function doTests() {
+      await testTabpanels();
+      await testPressed();
       SimpleTest.finish();
     }
 
@@ -94,5 +111,7 @@
         <hbox id="panel3"><button label="b3"/></hbox>
       </tabpanels>
     </tabbox>
+
+    <toolbarbutton id="toolbarbuttonCheckbox" type="checkbox">toolbarbuttonCheckbox</toolbarbutton>
   </hbox>
 </window>
diff --git a/accessible/tests/mochitest/states/test_controls.xhtml b/accessible/tests/mochitest/states/test_controls.xhtml
index de83f2aeccc01..a1997aef6ec37 100644
--- a/accessible/tests/mochitest/states/test_controls.xhtml
+++ b/accessible/tests/mochitest/states/test_controls.xhtml
@@ -32,7 +32,7 @@
       testStates("button", STATE_FOCUSABLE, 0, STATE_UNAVAILABLE);
       testStates("button-disabled", STATE_UNAVAILABLE, 0 , STATE_FOCUSABLE);
       testStates("checkbutton",
-                 STATE_FOCUSABLE | STATE_CHECKABLE | STATE_PRESSED);
+                 STATE_FOCUSABLE | STATE_PRESSED, 0, STATE_CHECKABLE);
       testStates("fakecheckbutton", STATE_FOCUSABLE | STATE_PRESSED, 0, 
                  STATE_CHECKABLE);
       testStates("combobox", STATE_FOCUSABLE | STATE_HASPOPUP, 0, STATE_UNAVAILABLE);
diff --git a/accessible/tests/mochitest/tree/test_button.xhtml b/accessible/tests/mochitest/tree/test_button.xhtml
index 67c81239f542e..fec453b71761f 100644
--- a/accessible/tests/mochitest/tree/test_button.xhtml
+++ b/accessible/tests/mochitest/tree/test_button.xhtml
@@ -40,6 +40,16 @@
       };
       testAccessibleTree("button2", accTree);
 
+      //////////////////////////////////////////////////////////////////////////
+      // toolbarbutton with type="checkbox"
+
+      accTree = {
+        role: ROLE_TOGGLE_BUTTON,
+        name: "hello",
+        children: [ ]
+      };
+      testAccessibleTree("button3", accTree);
+
       SimpleTest.finish()
     }
 
@@ -65,6 +75,7 @@
     <vbox flex="1">
       <button id="button1" label="hello"/>
       <toolbarbutton id="button2" label="hello"/>
+      <toolbarbutton id="button3" type="checkbox" label="hello"/>
     </vbox>
   </hbox>
 
diff --git a/accessible/xul/XULFormControlAccessible.cpp b/accessible/xul/XULFormControlAccessible.cpp
index 8de5ee59a1041..7c5ca1c4b2630 100644
--- a/accessible/xul/XULFormControlAccessible.cpp
+++ b/accessible/xul/XULFormControlAccessible.cpp
@@ -58,7 +58,19 @@ void XULButtonAccessible::ActionNameAt(uint8_t aIndex, nsAString& aName) {
 ////////////////////////////////////////////////////////////////////////////////
 // XULButtonAccessible: LocalAccessible
 
-role XULButtonAccessible::NativeRole() const { return roles::PUSHBUTTON; }
+role XULButtonAccessible::NativeRole() const {
+  // Buttons can be checked; they simply appear pressed in rather than checked.
+  // In this case, we must expose them as toggle buttons.
+  nsCOMPtr<nsIDOMXULButtonElement> xulButtonElement = Elm()->AsXULButton();
+  if (xulButtonElement) {
+    nsAutoString type;
+    xulButtonElement->GetType(type);
+    if (type.EqualsLiteral("checkbox") || type.EqualsLiteral("radio")) {
+      return roles::TOGGLE_BUTTON;
+    }
+  }
+  return roles::PUSHBUTTON;
+}
 
 uint64_t XULButtonAccessible::NativeState() const {
   // Possible states: focused, focusable, unavailable(disabled).
@@ -66,14 +78,8 @@ uint64_t XULButtonAccessible::NativeState() const {
   // get focus and disable status from base class
   uint64_t state = LocalAccessible::NativeState();
 
-  // Buttons can be checked -- they simply appear pressed in rather than checked
   nsCOMPtr<nsIDOMXULButtonElement> xulButtonElement = Elm()->AsXULButton();
   if (xulButtonElement) {
-    nsAutoString type;
-    xulButtonElement->GetType(type);
-    if (type.EqualsLiteral("checkbox") || type.EqualsLiteral("radio")) {
-      state |= states::CHECKABLE;
-    }
     // Some buttons can have their checked state set without being of type
     // checkbox or radio. Expose the pressed state unconditionally.
     bool checked = false;
@@ -92,6 +98,13 @@ uint64_t XULButtonAccessible::NativeState() const {
   return state;
 }
 
+bool XULButtonAccessible::AttributeChangesState(nsAtom* aAttribute) {
+  if (aAttribute == nsGkAtoms::checked) {
+    return true;
+  }
+  return AccessibleWrap::AttributeChangesState(aAttribute);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // XULButtonAccessible: Widgets
 
diff --git a/accessible/xul/XULFormControlAccessible.h b/accessible/xul/XULFormControlAccessible.h
index 1f5753d291917..e48b0249a2a5f 100644
--- a/accessible/xul/XULFormControlAccessible.h
+++ b/accessible/xul/XULFormControlAccessible.h
@@ -32,6 +32,7 @@ class XULButtonAccessible : public AccessibleWrap {
   // LocalAccessible
   virtual mozilla::a11y::role NativeRole() const override;
   virtual uint64_t NativeState() const override;
+  virtual bool AttributeChangesState(nsAtom* aAttribute) override;
 
   // ActionAccessible
   virtual bool HasPrimaryAction() const override;