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

Jump to bottom

v2.2.0 role=doc-toc does not follow the spec #653

Closed
XhmikosR opened this issue Apr 23, 2020 · 7 comments
Closed

v2.2.0 role=doc-toc does not follow the spec #653

XhmikosR opened this issue Apr 23, 2020 · 7 comments
Assignees
@gettalong
Labels
bug

Comments

@XhmikosR
Copy link

@XhmikosR XhmikosR commented Apr 23, 2020

Hello!

I just updated locally to kramdown v2.2.0 and I'm getting a new HTML validation error.

Bad value “doc-toc” for attribute “role” on element “ul”.

So, it seems the specs only allow role=doc-toc for nav and section but not ul.

I can ignore the warning, but I thought I'd mention the issue.
@sideshowbarker
Copy link

@sideshowbarker sideshowbarker commented Apr 23, 2020

Maintainer of the W3C HTML checker (validator) here. I can confirm that using role=doc-toc for the ul element isn’t valid. For checking ARIA roles, the W3C checker follows the rules in the ARIA in HTML spec at https://w3c.github.io/html-aria/ — and that spec says role=doc-toc may only be used with the nav and section elements.

@gettalong gettalong self-assigned this Apr 23, 2020
@gettalong gettalong added the bug label Apr 23, 2020
@gettalong
Copy link
Owner

@gettalong gettalong commented Apr 23, 2020

Thanks for bringing this to my attention - I have released version 2.2.1 that fixes this problem.

@XhmikosR
Copy link
Author

@XhmikosR XhmikosR commented Apr 23, 2020 •
edited

@gettalong Thanks! Might be worth checking the rest of the doc-* additions. I didn't check them because my test site doesn't use them.

@MaximeKjaer
Copy link
Contributor

@MaximeKjaer MaximeKjaer commented May 8, 2020 •
edited

Hi! I'm the person who contributed the change with ARIA roles, and thereby introduced this bug. I'd like to look into adding HTML validation as part of the CI to avoid the introduction of such bugs in the future.

@gettalong would you accept a PR in that direction?

It seems that doc-endnotes might only be allowed for <section>, but I am not sure that my reading of the spec is correct. Kramdown currently assigns it to <div>s, so that could be another instance of this problem. To be sure of it, I'd like to confirm this with an HTML validator first.

@XhmikosR did you use the W3C HTML Validator?

@MaximeKjaer
Copy link
Contributor

@MaximeKjaer MaximeKjaer commented May 8, 2020

I did try the W3C HTML checker with the following input with doc-endnotes (corresponding to one of the test cases, wrapped in an HTML page with <html>, <head> and <body>), but it did not raise an error, so I suppose that this usage is OK.

Tested document 
<!DOCTYPE html>
<html lang="en">
<head><title>Test</title></head>
<body>
<h1>No header id</h1>

<h1>without header id</h1>

<div>
some <span>*para*</span>
</div>

<div>
  <p>some <span><em>para</em></span></p>
</div>

<p>Some text<sup id="fnref:ab" role="doc-noteref"><a href="#fn:ab" class="footnote">10</a></sup>.</p>

<div class="footnotes" role="doc-endnotes">
  <ol start="10">
    <li id="fn:ab" role="doc-endnote">
      <p>Some text. <a href="#fnref:ab" class="reversefootnote" role="doc-backlink">&#8617;</a></p>
    </li>
  </ol>
</div>
</body>
</html>

@gettalong
Copy link
Owner

@gettalong gettalong commented May 8, 2020

@MaximeKjaer Thank you for chiming in and following up on this! Yes, I would accept such a PR.

@XhmikosR
Copy link
Author

@XhmikosR XhmikosR commented May 9, 2020

@MaximeKjaer I only use the official validator, mostly through its npm package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gettalong gettalong

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@gettalong @sideshowbarker @XhmikosR @MaximeKjaer