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

"ARIA required context role" and "ARIA required owned elements" (ff89c9 / bc4a75): Update note on DPUB due to upcoming changes in 1.1 #1473

Merged

Conversation

Jym77
Copy link
Collaborator

@Jym77 Jym77 commented Oct 8, 2020

The inapplicable example about ARIA DPUB role was failing the requirement because the doc-biblioentry role was not owned by its required doc-bibliography. This resulted as implementation considering the ARIA DPUB module (such as Alfa) to be rejected as they were failing this example.

Closes issue(s):

  • N/A

Need for Final Call:
This will require a 1 week Final Call (updating an example)


Pull Request Etiquette

When creating PR:

  • Make sure you're requesting to pull a branch (right side) to the develop branch (left side).

After creating PR:

  • Add yourself (and co-authors) as "Assignees" for PR.
  • Add label to indicate if it's a Rule, Definition or Chore.
  • Link the PR to any issue it solves. This will be done automatically by referencing the issue at the top of this comment in the indicated place.
  • Optionally request feedback from anyone in particular by assigning them as "Reviewers".

When merging a PR:

  • Close any issue that the PR resolves. This will happen automatically upon merging if the PR was correctly linked to the issue, e.g. by referencing the issue at the top of this comment.

How to Review And Approve

  • Go to the “Files changed” tab
  • Here you will have the option to leave comments on different lines.
  • Once the review is completed, find the “Review changes” button in the top right, select “Approve” (if you are really confident in the rule) or "Request changes" and click “Submit review”.
  • Make sure to also review the proposed Final Call period. In case of disagreement, the longer period wins.

@Jym77 Jym77 added Rule Update Use this label for an existing rule that is being updated reviewers wanted labels Oct 8, 2020
@Jym77 Jym77 self-assigned this Oct 8, 2020
kasperisager
kasperisager previously approved these changes Oct 8, 2020
carlosapaduarte
carlosapaduarte previously approved these changes Oct 8, 2020
Comment on lines 214 to 216
<div role="list">
<p role="doc-biblioentry" id="b8cab5dd-bc24-459c-9858-7afa9da69b64">
John Steinbeck, The Grapes of Wrath (New York: The Viking Press, 1939)
</p>
</div>
<p role="doc-biblioentry" id="b8cab5dd-bc24-459c-9858-7afa9da69b64">
John Steinbeck, The Grapes of Wrath (New York: The Viking Press, 1939)
</p>
Copy link
Member

Choose a reason for hiding this comment

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

This should not be changed. The point of this example is to demonstrate that "owned by" as used in ARIA means any descendant, not just the child. This example comes directly from the DPUB ARIA spec:

https://www.w3.org/TR/dpub-aria-1.0/#doc-biblioentry

This confusion about "owned by" including all descendants, and not just the children and direct references of aria-owns is a known issue in ARIA. See: w3c/aria#1161, w3c/aria#748.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🙈 That example has so many problems I want to unsee it…

Fortunately (maybe), doc-biblioentry is deprecated in DPUB-ARIA 1.1 (https://w3c.github.io/dpub-aria/#doc-biblioentry) precisely because of the wonkyness it creates with the list role (w3c/dpub-aria#15 and w3c/dpub-aria#22). This is very recent changes!

I guess this example should be completely revamped. As far as I can tell, there is no more required context roles in DPUB-ARIA 1.1. I don't think it is a good idea to keep an example showing a known problem in the specs which is already fixed in the next version.

If the point of the example is to show the "descendant" meaning of "owned by", we need to find a non DPUB example.

If the point of the example is to show that DPUB roles are ignored, we can take any, but they have no required context role…

Maybe we should update the Background note saying that DPUB-ARIA 1.1 does not have any required context role (like Graphics ARIA) which justifies our choice to restrict ourselves to ARIA 1.1 roles.

@Jym77
Copy link
Collaborator Author

Jym77 commented Oct 15, 2020

I've updated the background explaining why we focus on ARIA. Now saying that DPUB 1.1 has no role with required context role. Copied that note to "required owned element" rule where the same thing happen. Essentially, the "unresolved issues" that this note mentioned have been resolved by deprecating the offending elements.

The example has been updated accordingly.

Dismissing all reviews, since this is a significant change of scope.

@Jym77 Jym77 changed the title "ARIA required context role" (ff89c9): Make inapplicable example not fail the requirement "ARIA required context role" and "ARIA required owned elements" (ff89c9 / bc4a75): Update note on DPUB due to upcoming changes in 1.1 Oct 15, 2020
@Jym77 Jym77 dismissed carlosapaduarte’s stale review October 15, 2020 11:53

Significant changes.

@Jym77 Jym77 requested a review from carlosapaduarte October 15, 2020 11:53
@Jym77 Jym77 dismissed kasperisager’s stale review October 15, 2020 11:53

Significant changes.

@Jym77 Jym77 requested a review from kasperisager October 15, 2020 11:53
@Jym77 Jym77 dismissed WilcoFiers’s stale review October 15, 2020 11:54

Adapted to DPUB 1.1

@Jym77 Jym77 requested a review from WilcoFiers October 15, 2020 11:54
@@ -43,7 +43,7 @@ This rule assumes that the `role` attribute is used to give a [semantic role][]

## Background

The applicability of this rule is limited to the [WAI-ARIA 1.1 Recommendation][aria 1.1] roles, since there are unresolved issues with how [Digital Publishing WAI-ARIA Module (DPUB ARIA) 1.0][dpub 1.0] uses role inheritance to define the [required context roles][], which makes it deviate from the model defined in [WAI-ARIA 1.1][aria 1.1]. The [WAI-ARIA Graphics Module](https://www.w3.org/TR/graphics-aria-1.0/) does not include any [required context roles][].
The applicability of this rule is limited to the [WAI-ARIA 1.1 Recommendation][aria 1.1] roles. The [WAI-ARIA Graphics Module][] does not include any [required context roles][]. The [Digital Publishing WAI-ARIA Module (DPUB ARIA) 1.0][dpub 1.0] only has two roles with [required context roles][] (`doc-biblioentry` and `doc-endnote`); both of them have issues with their use of role inheritance, and both of them are deprecated in the [Digital Publishing WAI-ARIA Module (DPUB ARIA) 1.1][dpub 1.1] editor's draft.
Copy link
Member

Choose a reason for hiding this comment

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

DPUB-ARIA 1.1 hasn't even made it to first draft yet. I don't think we should be referencing it.

Copy link
Member

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

I think this is getting ahead of things, and I don't think the new example is a proper replacement of the current one.

@Jym77 Jym77 dismissed WilcoFiers’s stale review October 20, 2020 13:33

Example fails accessibility requirement and should be killed anyway

@Jym77 Jym77 requested a review from WilcoFiers October 20, 2020 13:33
@Jym77
Copy link
Collaborator Author

Jym77 commented Oct 28, 2020

Final Call ends on November 4.

@Jym77 Jym77 added Review Call 1 week Call for review for small changes and removed reviewers wanted labels Oct 28, 2020
@Jym77
Copy link
Collaborator Author

Jym77 commented Nov 5, 2020

Final Call has ended. Merging.

@Jym77 Jym77 merged commit cb930b0 into develop Nov 5, 2020
@Jym77 Jym77 deleted the aria-required-context-role-inapplicable-example-passes-mapping branch November 5, 2020 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review Call 1 week Call for review for small changes Rule Update Use this label for an existing rule that is being updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants