Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

empty-table-header virtualised rule should run without requiring role #3788

Closed
WilcoFiers opened this issue Nov 17, 2022 · 3 comments
Closed
Assignees
Labels
fix Bug fixes
Milestone

Comments

@WilcoFiers
Copy link
Contributor

This test needs to be added, and needs to pass.

  it('should fail if table header has no child nodes', function () {
    var node = new axe.SerialVirtualNode({
      nodeName: 'th',
      attributes: {}
    });
    node.children = [];

    var results = axe.runVirtualRule('empty-table-header', node);

    assert.lengthOf(results.passes, 0);
    assert.lengthOf(results.violations, 1);
    assert.lengthOf(results.incomplete, 0);
  });
@WilcoFiers WilcoFiers added the fix Bug fixes label Nov 17, 2022
@dbowling
Copy link
Contributor

@WilcoFiers - as I was looking at this tonight, I see we actually already have this test as part of my original PR, but our expectation was that it incompletes:

it('should incomplete when children are missing', function () {
    var thNode = new axe.SerialVirtualNode({
      nodeName: 'th'
    });
    thNode.children = [];

    var results = axe.runVirtualRule('empty-table-header', thNode);

    assert.lengthOf(results.passes, 0);
    assert.lengthOf(results.violations, 0);
    assert.lengthOf(results.incomplete, 1);
  });

Note: existing test is has attributes: {} vs attributes: undefined, and has different asserts for results.violations and results.incomplete.

The test was based on a suggestion from Steve:

We'll want a test that should return incomplete if the th element does not have the children set to an empty array. Similar to button-name

TTBOMK, the empty object vs undefined for attributes isn't something that this code should differentiate on, so I'm not seeing how things need to change. Let me know if that's not the case and I will look with fresh eyes, and maybe we can chat about how to make a test that is more expressive about intent.

@straker
Copy link
Contributor

straker commented Nov 23, 2022

We dug into this. The reason this test is not failing is due to the role algorithm looking up the parent tree of the th looking for presentational parents. That is because if the table parent was presentational, the entire table is and we don't want to run the empty-table-header rule on it.

The fix is to not throw on that line if we don't have an actualNode, which will then allow the test to fail as expected.

Looking at this we also discovered that the empty-table-header rule is looking at th elements even if their role is changed, so we should update the selector to exclude th elements with a role: th:not([role])

@padmavemulapati
Copy link

Validated with the latest code base, axe-core develop branch:
for the test snippet:

<table>
    <tr>
      <th id="ignore1" role="spinbutton">rowheader with a role</th>
      <th id="ignore1" role="none">rowheader with a role</th>
      <th id="ignore1" role="cell">rowheader with a role</th>
    </tr>
  </table>

empty-table-header fails when role="spinbutton" , pass when role="none" or role="cell"

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fixes
Projects
None yet
Development

No branches or pull requests

4 participants