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

"Heading has non-empty accessible name" [ffd0e9]: Update from TF survey #2077

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions _rules/heading-non-empty-accessible-name-ffd0e9.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ accessibility_requirements:
failed: not satisfied
passed: further testing needed
inapplicable: further testing needed
secondary_requirements:
wcag20:1.3.1: # Info and Relationships (A)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the correct syntax. What you should do is replace:

  wcag20:1.3.1: # Info and Relationships (A)
    forConformance: true
    failed: not satisfied
    passed: further testing needed
    inapplicable: further testing needed

with this:

  wcag20:1.3.1: # Info and Relationships (A)
    secondary: true

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WilcoFiers am I to update within this PR or do I need to create a new one with the desired changes you recommend per secondary requirements? Not sure if you wanted another PR on 7fb7a37 for the highlight you are mentioning in this comment here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No please update in this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WilcoFiers when I view the file in "edit file" it shows your changes. I can't approve per admin settings, thus not sure what else to do to "close" this issue and "update the PR" from my side.

wcag-technique:H42: # Using h1-h6 to identify headings
forConformance: false
failed: not satisfied
Expand Down Expand Up @@ -40,7 +42,7 @@ There are no assumptions.

## Accessibility Support

- Some assistive technologies may hide headings with empty [accessible name][] from the users. This depends on the user agent, on how the [accessible name][] was computed (the [accessible name and description computation][] is not clear concerning which characters should be trimmed), and on the assistive technology itself. Hence, there are cases where the outcome of this rule is _failed_, but users of certain assistive technology and browser combinations will not experience an issue.
- Some assistive technologies may hide headings with empty [accessible name][] from the users. This depends on the user agent, on how the [accessible name][] was computed (the [accessible name and description computation][] and on the assistive technology itself. Hence, there are cases where the outcome of this rule is _failed_, but users of certain assistive technology and browser combinations will not experience an issue.
ChrisLoiselle marked this conversation as resolved.
Show resolved Hide resolved

- Implementation of [Presentational Roles Conflict Resolution][] varies from one browser or assistive technology to another. Depending on this, some [semantic][semantic role] `heading` elements can fail this rule with some technology but users of other technologies would not experience any accessibility issue because the same elements would have a [semantic role][] of `presentation` and be hidden for these users.

Expand Down Expand Up @@ -173,14 +175,6 @@ This `h1` element has an [explicit role][] of `none`. However, the [global][] [p

#### Inapplicable Example 1

There is no [semantic][semantic role] `heading` element.

```html
<div></div>
```
Comment on lines -176 to -180
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this example breaks our own "test case design" guidelines of Test every condition 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jym77 We felt this was not a useful test case, so we're thinking of leaving things like this out. Is that OK with you? Happy to update the test ever condition bit then.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WilcoFiers No strong feeling about that particular example which is, indeed, quite useless 🙂

It's more on the boundaries of things on how we decide that tests are useful or not.
From a TDD point of view, we should indeed have test cases for every condition, and since we do use the test cases to validate tools, this also sounds like a good idea from an engineering point of view.
From an "illustrate the rule to human" point of view, this is indeed a fairly useless case…

=> this tends to hint that we may want to have some test cases for the computers and only show a meaningful subset to the humans.


#### Inapplicable Example 2

This `h1` element is not [included in the accessibility tree][].

```html
Expand Down