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

Update html-aam/roles-contextual.html with <summary> tests #43859

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

rahimabdi
Copy link
Contributor

This PR adds additional tests for <summary> role calculation per html-aam <summary> mapping guidance:

If a summary element is not a child of a details element, or it is not the first summary element of a parent details, then the summary element MUST be exposed with a generic role.

<li>x</li>
<li>x</li>
</ol>
<summary data-testname="el-summary-not-first-child-of-details" class="ex-generic">x</summary>
Copy link
Contributor

@scottaohara scottaohara Jan 5, 2024

Choose a reason for hiding this comment

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

this wouldn't be a generic, because it's the first summary that is a child of the details element.

so

<details>
  whatever text content or other non-summary HTML elements can go here
  <div>
    <summary> this is not a child of the details, so it is a generic </summary>
  </div>
  <summary> this is not a generic because it's the first child summary of the details parent</summary>
  <summary> this is a generic because it's the second child summary of the details parent </summary>
</details>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed (I had mixed up the requirement for <summary> to be the first child of <details> which throws a parsing error).

@cookiecrook
Copy link
Contributor

Since there are some special rules for Summary, I think it should be its own file rather than split between roles.html and roles-contextual.html.

Another test that is probably missing (and may need spec clarification:

  • @rahimabdi if there is only one summary descendant, (and it’s rendered when details is collapsed) it should be used regardless if it’s a child or not. (Prioritize users/authors over spec purity) The first child test should only become a factor when there are multiple summary elements in a single details. (I don’t want the fix for this edge case to break the accessibility of a markup snippet that “works” in the mainstream UI.)
  • @scottaohara i didn’t look back at html or html-aam, but that distinction is not in your excerpt above. Instead of making the AAM spec distinction, shouldn’t we defer/link to the HTML spec and expose the summary of whatever they determine is actually rendered?

@scottaohara
Copy link
Contributor

@cookiecrook if i understand your comment, there is no author defined summary as a direct child of the details, then browsers auto inject a summary as the first child.

so per the following:

<details>
  <div><summary>foo</summary></div>
</details>

the the above summary within the div will be a generic because the browser will inject a new summary element prior to that div (with the localized text of 'details')

@rahimabdi
Copy link
Contributor Author

rahimabdi commented Jan 6, 2024

@scottaohara @cookiecrook May we add this as an agenda item for a future meeting? Happy to take this forward although I have several questions now 🙂

@cookiecrook
Copy link
Contributor

Okay I think @scottaohara explained sufficiently that the other non-child descendants are error-handled by some logic defined in the HTML spec, so the tests are fine, but instead of (or in addition too) this note in HTML-AAM:

If a summary element is not a child of a details element, or it is not the first summary element of a parent details, then the summary element MUST be exposed with a generic role.

…Add a link to this section of the HTML spec defining that behavior:

  1. If parent's first summary element child is not this summary element, then return false.

@cookiecrook
Copy link
Contributor

So I don’t need to meet Rahim (seems this is resolving), but you can add any issue to the meeting agenda by adding the Agenda label.

<li>x</li>
<li>x</li>
</ol>
<summary>x</summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not test this, the only semantic summary example?

<li>x</li>
</ol>
<summary>x</summary>
<summary data-testname="el-summary-not-first-summary-child-of-details" class="ex-generic">x</summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea why this one fails in WebKit with the following?

FAIL message: promise_test: Unhandled rejection with value: "error: Action get_computed_role failed"

Copy link
Contributor Author

@rahimabdi rahimabdi Apr 7, 2024

Choose a reason for hiding this comment

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

I think it's due to an unused AriaUtils.verifyRolesBySelector(".ex") in the <script> block since we're only testing generic stuff. I've removed it in 333c3df, re-running CI tests now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like there's not an issue in Gecko/Chromium (perhaps WebKit is not exposing the <summary> in this context as an accessible object)?

@rahimabdi
Copy link
Contributor Author

@scottaohara @cookiecrook Thank you both, this PR is ready for another review.

I've created a new html-aam/summary.html file (and updated roles.html/roles-contextual.html with a documentation link referencing the new file).

@rahimabdi
Copy link
Contributor Author

rahimabdi commented Apr 8, 2024

@scottaohara @cookiecrook One more question about the HTML-AAM <summary> mapping:

"If the element is the first child of its type within a parent details element: html-summary"

I'm not certain what html-summary references here; is this an abstract role?

@scottaohara
Copy link
Contributor

@scottaohara @cookiecrook One more question about the HTML-AAM <summary> mapping:

"If the element is the first child of its type within a parent details element: html-summary"

I'm not certain what html-summary references here; is this an abstract role?

there is no direct ARIA role for a summary element at this time. Previously (somewhere in the ARIA minutes) we resolved to treat elements that didn't have or didnt have an agreed upon ARIA role to have 'platform' computed rules. So in this case html-summary. Where as video would have a platform computed role of html-video

does that help, or did i misunderstand your question?

@cookiecrook
Copy link
Contributor

And: w3c/core-aam@5d6012b#diff-0eb547304658805aad788d320f10bf1f292797b5e6d745a3bf617584da017051R367

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

Successfully merging this pull request may close these issues.

4 participants