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

Remove Core-AAM UA requirement to disallow orphaned role context. #2284

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cookiecrook
Copy link
Contributor

Remove Core-AAM UA requirement to disallow orphaned role context.

Closes #2166

Copy link

netlify bot commented Jul 17, 2024

Deploy Preview for wai-aria ready!

Name Link
🔨 Latest commit be75e58
🔍 Latest deploy log https://app.netlify.com/sites/wai-aria/deploys/669786f15a84eb0008bfcbc1
😎 Deploy Preview https://deploy-preview-2284--wai-aria.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

I'd like to understand the exceptions a bit better!

core-aam/index.html Show resolved Hide resolved
<div role="list"> <!-- computedrole returns "list" -->
&lt;div role="listitem"&gt; &lt;!-- computedrole returns "listitem" in the required context. --&gt;</pre
>
<pre>&lt;div role="listitem"&gt; &lt;!-- Author error: orphaned listitem. computedrole is unspecified. --&gt;</pre>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I think I should keep the second example.

Suggested change
<pre>&lt;div role="listitem"&gt; &lt;!-- Author error: orphaned listitem. computedrole is unspecified. --&gt;</pre>
<pre>
&lt;div role="listitem"&gt; &lt;!-- Author error: orphaned listitem. computedrole is unspecified. --&gt;
&lt;div role="list"&gt; &lt;!-- computedrole returns "list" --&gt;
&lt;div role="listitem"&gt; &lt;!-- computedrole returns "listitem" in the appropriate context. --&gt;</pre>

Comment on lines +554 to +555
in scenarios deemed by the implementation to be harmless. This unspecified permissiveness in how engines treat author role usage errors is occasionally overridden for a specific role in
other specs such as [[HTML-AAM]].
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
in scenarios deemed by the implementation to be harmless. This unspecified permissiveness in how engines treat author role usage errors is occasionally overridden for a specific role in
other specs such as [[HTML-AAM]].
in scenarios deemed by the implementation to be harmless. Please note that this permissiveness in how engines treat author role errors might be overridden in a language-specific mapping document such as [[HTML-AAM]].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spectranaut The Prettier bot broke the thread (because of the line breaks it needlessly added) but this is in response to your feedback suggestion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, this seems better to me!

Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

Approval with the change to list html-aam specifically

Comment on lines +554 to +555
in scenarios deemed by the implementation to be harmless. This unspecified permissiveness in how engines treat author role usage errors is occasionally overridden for a specific role in
other specs such as [[HTML-AAM]].
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, this seems better to me!

Copy link
Member

@scottaohara scottaohara left a comment

Choose a reason for hiding this comment

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

lgtm when the suggested changes are merged in

@spectranaut
Copy link
Contributor

@cookiecrook if you include your proposed changes I will land!

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

Successfully merging this pull request may close these issues.

Implementation concerns over Orphaned Role requirements in Core-AAM, HTML-AAM… and possibly elsewhere.
3 participants