Patch history and review: Phabricator D272807

Intro

As this bug had many previous attempts at solving it, its respective Bugzilla page was jam-packed. It’s always helpful to access information pertaining to the bug you’re trying to fix, but in large quantities it can be a little hard to digest. Fortunately, Eitan Isaacson (:eeejay), the bug’s mentor, did a great job guiding the efforts for the solution.

To understand this bug, let’s first delve into the concept of accessibility tree. The accessibility tree is a parallel representation of a web page that’s specifically designed for assistive technologies (like screen readers, voice control software, etc.). Like a simplified, semantic version of the DOM. Without the accessibility tree, screen readers would have to parse raw HTML/DOM, which contains too much noise. The tree provides a clean, semantic interface that assistive technologies can reliably consume.

As an example, consider this HTML:

  <div class="card-wrapper">
    <div class="card">
      <button id="btn">Click me</button>
    </div>
  </div>

Its DOM tree will be:

  div (class="card-wrapper")
    └─ div (class="card")
        └─ button#btn
            └─ "Click me"

And its (somewhat simplified) accessibility tree would look like:

  button
    ├─ role: button
    ├─ name: "Click me"
    ├─ state: focusable
    └─ state: enabled

Each node in the accessibility tree has:

  • Role - What type of element is this? (button, heading, table, cell, navigation, etc.)
  • Name - What’s it called? (button text, alt text, aria-label, etc.)
  • State - Current condition (checked, expanded, selected, focused, etc.)
  • Properties - Static attributes (readonly, required, level, etc.)
  • Relationships - Connections to other elements (labelledby, describedby, owns, etc.)

The Problem

When no ARIA roles are applied, a <td> element in an HTML table has the default role of cell. Notably, this role is absent from the “Used in Roles” values in the ARIA specification page. This means that by default, the aria-selected state value for cell nodes in the tree should be undefined. Having it false or true would imply they’re selectable.

The bug was that Firefox was making all table cells expose the SELECTABLE state in the accessibility tree. The aria-selected state values were defaulting to false, instead of undefined. This made screen readers announce every single table cell as “selectable” when navigating regular HTML tables (like Wikipedia tables). This problem emanates from code that adds states::SELECTABLE flag irrespective of any circumstance.

Solving this problem would involve 3 steps:

  1. Going to its root and making native HTML table cells NOT expose SELECTABLE state by default.
  2. Creating a test in the a11y test suite that confirms this behavior has been fixed.
  3. Checking if this broke any previous test and, if so, understanding why it did and fixing it.

Fixing Step-By-Step

1. Removing the override

The culprit code was located in the accessible/html/HTMLTableAccessible.cpp file:

uint64_t HTMLTableCellAccessible::NativeInteractiveState() const {
  return HyperTextAccessible::NativeInteractiveState() | states::SELECTABLE;
}

This code was basically ensuring that all HTML table cells were selectable. It defines the HTMLTableCellAccessible::NativeInteractiveState method to return the states of its parent class - that’s what the HyperTextAccessible::NativeInteractiveState() does - while adding the SELECTABLE flag to it - that’s what the bitwise OR operation with the states::SELECTABLE operand does. It was effectively overriding the NativeInteractiveState method for the HTMLTableCellAccessible class, forcing SELECTABLE onto every table cell at the “native” level, before ARIA even gets a chance to modify anything.

Removing the method definition also entailed removing its declaration from accessible/html/HTMLTableAccessible.h:

  virtual uint64_t NativeInteractiveState() const override;

2. Creating the test file

To prove that the bug was fixed, I needed to create a test covering its particular issue. The browser chrome testing suite establishes a series of conventions and ways for how these test files are written. After studying how they’re made, I came up with the following JavaScript test:

/**
 * Test that native HTML table cells are not selectable by default.
 */
addAccessibleTask(
  `
  <table id="native_table">
    <tr>
      <td id="native_cell">Native cell</td>
    </tr>
  </table>
  `,
  async function (browser, docAcc) {
    const nativeCell = findAccessibleChildByID(docAcc, "native_cell");
    testStates(nativeCell, 0, 0, STATE_SELECTABLE, 0);
  },
  { topLevel: true, iframe: true, remoteIframe: true, chrome: true }
);

It’s very simple and self-explanatory; it simply tests that a native HTML table cell is not going to have that SELECTABLE state set as true by default. This test now lives alongside the other browser chrome state tests in the accessible/tests/browser/states/ folder. I named it browser_test_table_selectable.js. I also declared it in the accessible/tests/browser/states/browser.toml file.

3. Fixing test regressions

After fixing this problem and devising a test to prove it, it was expected that some previously passing tests would fail. That’s because the tests were built around the erroneous behavior, i.e. of all native HTML table cells exposing SELECTABLE state by default. It was therefore paramount, for me, to run the test suite and see what tests would fail. I needed not only to confirm my code changes, but to find any test that eventually broke due to those changes. And to save everyone’s time I should better push the patch only after running the test suite successfully. Fortunately, I was able to overcome my hardware limitations and run the testing suite on my own.

Turns out only the assertions from a single file were needing attention. In the accessible/tests/mochitest/table/test_sels_table.html file from the mochitest suite there were assertions expecting that cell’s aria-selectable state value should default to false - although this is an HTML file, the tests are run through JavaScript code within script tags. With this patch they should now default to undefined. It bears reminding that the change now complies with the ARIA specification:

ValueDescription
falseThe selectable element is not selected.
trueThe selectable element is selected.
undefined (default)The element is not selectable.

At the code level all that was needed was changing the values from false to undefined on a couple arrays assigned to variables named cellArray.

Takeaways

I’m proud of being responsible for nudging Firefox in the direction of increasing its accessibility support. This fix brings Firefox closer to full ARIA specification compliance, ensuring that native table cells behave as the standard intended. When browsers align on accessibility semantics, assistive technology developers can build more reliable tools, and users get a more consistent experience across the web.

Beyond the fix itself, I gained a deeper understanding of how browsers expose semantic information to assistive technologies — the accessibility tree, ARIA roles and states, and how to navigate a massive C++ codebase despite never having written C++ before. Navigating inheritance hierarchies, understanding bitwise state flags, and tracing method calls through Mozilla’s codebase reinforced something I’ve come to trust: a solid foundation in programming fundamentals transfers across languages. The syntax is just the surface. Diving into Mozilla’s C++ codebase and shipping a working patch proved that just-in-time learning works when you’ve built a solid foundation.