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

There is no way to trigger the layout table rule to raise anything #1852

Closed
dylanb opened this issue Oct 16, 2019 · 10 comments
Closed

There is no way to trigger the layout table rule to raise anything #1852

dylanb opened this issue Oct 16, 2019 · 10 comments
Assignees
Labels
docs Documentation changes rules Issue or false result from an axe-core rule tech debt Technical debt related tasks
Milestone

Comments

@dylanb
Copy link
Contributor

dylanb commented Oct 16, 2019

Expectation: a layout table with a caption should raise an issue

Actual: no issues raised

Motivation: rules should work as advertised


axe-core version: 3.3.2
axe extension: 4.0.0

Table markup used to try to generate the issue

            <table role="presentation">
                <caption>presentation caption</caption>
                <tr><td>hello</td></tr>
            </table>

I also noticed that there are no integration tests for this rule.

@dylanb dylanb added fix Bug fixes rules Issue or false result from an axe-core rule labels Oct 16, 2019
@WilcoFiers
Copy link
Contributor

WilcoFiers commented Oct 18, 2019

This is intentional, it was done to close this false positive: #561

I agree with these guys that this shouldn't be a WCAG failure. Putting role="presentation" on a table is a trick people use to force AT to ignore it, even if that table has markup like th / captions / scope="".

I think there's a decent case to be made that this should be failed as part of a best practice. People shouldn't write tables like that, but if you have a table that you need to fix, this is a valid way of doing it. Failing this under the existing rules would be a false positive.

@WilcoFiers WilcoFiers added needs discussion More discussion is needed to continue tables and removed fix Bug fixes labels Oct 18, 2019
@robdodson
Copy link
Contributor

@WilcoFiers do you have any examples of a table that would fail the existing rule? We're trying to decide if we should keep it in Lighthouse.

@WilcoFiers
Copy link
Contributor

@robdodson there are a few table rules, which ones were you asking about specifically? There are examples in the integration tests. Does that help? https://github.com/dequelabs/axe-core/tree/develop/test/integration/rules

@robdodson
Copy link
Contributor

Specifically asking about layout-table. I was wondering if the snippet dylan shared above should show up as a violation? Dylan mentioned he was able to trigger it in code because the rule is off by default. He said that it showed up in the incomplete array. Is it expected that layout-table would only ever show up in incomplete or are there some situations where it shows up in violations?

For context, we're trying to decide if we want to keep this rule turned on in Lighthouse because we're unsure how to actually fail it so it makes testing hard.

@WilcoFiers
Copy link
Contributor

@robdodson nope, Dylan's snippet shouldn't fail that. When it has role=presentation, tables get ignored by screen readers, except if it's focusable. Problems happen when people don't put a role on their layout table, and than also use caption, th, or summary="...".

We'll add some integration tests before the next release. I don't know how that got overlooked.

@WilcoFiers WilcoFiers added this to the Axe-core 3.5 milestone Oct 28, 2019
@WilcoFiers WilcoFiers added tech debt Technical debt related tasks and removed needs discussion More discussion is needed to continue labels Oct 28, 2019
@straker straker self-assigned this Nov 4, 2019
@straker
Copy link
Contributor

straker commented Nov 4, 2019

So after some testing, it seems dylan is correct and no table gets past the rule-matcher. Here is all the tables I tried and all but the last one won't pass the matcher.

<table id="ignore1">
  <caption>My table</caption>
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="ignore2" summary="My table">
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="ignore3">
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="ignore4" role="presentation">
  <caption>My table</caption>
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="ignore5" role="none">
  <caption>My table</caption>
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="ignore6" role="presentation" summary="My table">
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="ignore7" role="none" summary="My table">
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="ignore8" role="presentation">
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="ignore9" role="none">
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="fail1" role="presentation" tabindex="0">
  <caption>My table</caption>
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="fail2" role="none" tabindex="0">
  <caption>My table</caption>
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="fail3" role="presentation" summary="My table" tabindex="0">
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="fail4" role="none" summary="My table" tabindex="0">
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="fail5" role="presentation" tabindex="0">
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="fail6" role="none" tabindex="0">
  <thead>
    <tr>
      <th>Table heading 1</th>
      <th>Table heading 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Table row heading</th>
      <td>Cell</td>
    </tr>
  </tbody>
</table>

<table id="fail7" role="presentation" tabindex="0"></table>

The issue seems to be that in the matcher, we check to see if the table is a data table, which passes all tables that have a caption, or summary, or thead/tbody/tfoot, or table cells, unless they are empty or have role=presentation/none and aren't focusable.

However, the next part of the matcher then doesn't pass for elements that have role=presentation/none and aren't focusable. So effectively the matcher will never return true unless the table is empty, in which case it won't fail the checks.

@straker
Copy link
Contributor

straker commented Nov 6, 2019

We should deprecate this rule as it will never run.

@padmavemulapati
Copy link

seeing best-practice issues only when implementing role attribute.

image

@padmavemulapati
Copy link

@straker , is anything need to verify here..

@straker
Copy link
Contributor

straker commented Dec 3, 2019

Nope, we deprecated the rule so no testing needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation changes rules Issue or false result from an axe-core rule tech debt Technical debt related tasks
Projects
None yet
Development

No branches or pull requests

6 participants