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

Example 3 is wrong #65

Closed
JAWS-test opened this issue Oct 30, 2019 · 9 comments · Fixed by #206
Closed

Example 3 is wrong #65

JAWS-test opened this issue Oct 30, 2019 · 9 comments · Fixed by #206

Comments

@JAWS-test
Copy link

JAWS-test commented Oct 30, 2019

https://w3c.github.io/accname/#example-3:

<div role="checkbox" aria-checked="false">Flash the screen <span role="textbox" aria-multiline="false"> 5 </span> times</div>

According to https://www.w3.org/TR/wai-aria-1.1/#checkbox a checkbox has: "Children Presentational: True". Therefore the checkbox cannot contain an input field.

In my opinion, that would be correct:

<div role="checkbox" aria-checked="false" aria-labelledby="label"></div>
<div id="label">Flash the screen <span role="textbox" aria-multiline="false"> 5 </span> times</div>

For the example to be correct, both interactive elements need a tabindex=0 and the textbox a label. However, the following example would be simpler:

<div role="checkbox" tabindex="0" aria-checked="false" aria-labelledby="label"></div>
<label id="label">Flash the screen <input type="text" value="5"> times</label>
@ZoeBijl
Copy link

ZoeBijl commented Oct 30, 2019

Nice find! Can you make a PR that includes your first suggestion?

Your simpler example would be more complex for the user: while the label element provides a visible label for both controls it will activate the textbox rather than the checkbox. Checkboxes generally being small it would be better to have the label activate that instead of the textbox.

@JAWS-test
Copy link
Author

JAWS-test commented Oct 30, 2019

@ZoeBijl

Your simpler example would be more complex for the user: while the label element provides a visible label for both controls it will activate the textbox rather than the checkbox. Checkboxes generally being small it would be better to have the label activate that instead of the textbox.

I'm afraid I don't understand your argument. In the first example, the checkbox does not have a label that activates the checkbox (unless there is an additional JS). If you think the click area of the label is important, maybe it would be better to use the following

<label id=label1><input type="checkbox" aria-labelledby="label1 label2"> Flash the screen</label>
<label id=label2><input type="text" value="5"> times</label>

If we've agreed on a variant, I can make a PR.

@jnurthen jnurthen added this to the 1.2 milestone Oct 31, 2019
JAWS-test added a commit to JAWS-test/accname that referenced this issue Nov 6, 2019
@jnurthen jnurthen reopened this Nov 8, 2019
@jnurthen
Copy link
Member

jnurthen commented Nov 8, 2019

Please leave open until the fix gets merged.

@JAWS-test
Copy link
Author

Okay, thanks for the info. I thought the issue could be closed once a PR was created

@jnurthen
Copy link
Member

jnurthen commented Nov 8, 2019

as You put the keyword “fixes #xx” or “closes #xx” in the PR it will automatically close the issue when the PR gets merged.

@ZoeBijl
Copy link

ZoeBijl commented Nov 11, 2019

I think we can simplify the code to this:

<input type="checkbox" id="checkbox">
<label for="checkbox">Flash the screen <span role="textbox" contenteditable>5</span> times</label>

This way the label points to the checkbox which makes it a lot easier to toggle. I have removed the aria-multiline from the role="textbox" element as it defaults to false anyway.

The textbox doeesn’t have a label. But I would assume this to pass WCAG §3.3.2 under G167 Using an adjacent button to label the purpose of a field.

There’s a CodePen with this code for testing. Also available as debug view.

@JAWS-test
Copy link
Author

The textbox doeesn’t have a label.

Input field (role=textbox) without name does not violate SC 3.3.2, but violates SC 1.3.1 and SC 4.1.2.

However, I do not know whether it is important that the example conforms to WCAG. It's just about explaining how the checkbox labeling works.

@accdc
Copy link
Contributor

accdc commented Jun 11, 2021

I agree the example is contradictory due to the 'children presentational' aspect of the parent role, but what this test is meant to represent is not valid markup or recommended functionality for use in the wild, but rather, what the recursive algorithm is meant to do when it encounters convoluted markup structures that include an embedded widget role that supports an implicit value.

In this case, the test matches many of the edge cases documented in the W3C AccName test page at
https://www.w3.org/wiki/AccName_1.1_Testable_Statements

If you review the tests there, you will notice many examples where developers should never do this in practice, however the purpose of these tests are meant to test the robustness of the AccName algorithm so it can successfully handle critical edge cases as well as possible when it encounters seeming contradictions and oddly nested markup structures.

Think of these as stress tests to break the algorithm if anything goes wrong.

If we only gear the test cases to only include perfectly workable and valid markup, we will miss all of the cases where the markup is not perfect, and it will lead to gaping holes in the reliability of the algorithm when it needs to be capable of recovering gracefully as much as possible.

To that end, if we change the test over much, it changes the outcome and the way the algorithm is meant to handle it, thus invalidating its ability to recover from similar conditions when encountered in the wild.

If the ARIA WG is in agreement that this needs to be changed to something else for the purpose of illustration, we can certainly do that. @jnurthen Can this be added as a topic for us to talk about while speaking about AccName next week?

Just please keep in mind that the test cases for algorithm robustness are not actually meant to be directly consumable in most cases as working examples that fully comply with all accessibility checkers and standards.

@cookiecrook
Copy link
Contributor

AS an FYI, the diff in #206 uses a native checkbox and label combo which, unlike the suggested example, retains label clickability. However since including two <input> elements in the <label> makes it invalid, the interior field is a custom ARIA textbox. I believe this still solves the original childrenPresentational issue, since the second field is contained within the label, not the checkbox.

<label for="flash">
  <input type="checkbox" id="flash">
  Flash the screen <span tabindex="0" role="textbox" aria-label="number of times" contenteditable>5</span> times.
</label>

But if you notice another problem, please comment in the PR:

@accdc accdc closed this as completed in #206 Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants