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.

Sign up for GitHub

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

role=heading missing from heading outline #308

Closed
pkra opened this issue Mar 3, 2020 · 6 comments · Fixed by #309
Closed

role=heading missing from heading outline #308

pkra opened this issue Mar 3, 2020 · 6 comments · Fixed by #309
Assignees
@rdeltour
Labels
bug

Comments

@pkra
Copy link
Contributor

pkra commented Mar 3, 2020

I'm using Ace v1.1.1 on node v12.10 (ubuntu 19.10).

Elements with role=heading (and, say, aria-level=1) do not seem to be included in the heading outline.

E.g.,

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml" xmlns:epub="http://www.idpf.org/2007/ops" lang="en" xml:lang="en">
<head>
<title>Ace role heading test</title>
</head>
<body>
<div role="heading" aria-level="1">An H1!</div>
<p>Hello</p>
</body>
</html>

leads to "Untitled BODY" in the outlines.
@rdeltour
Copy link
Member

rdeltour commented Mar 3, 2020

Elements with role=heading (and, say, aria-level=1) do not seem to be included in the heading outline.

Good catch, thanks for the report!

@rdeltour rdeltour self-assigned this Mar 3, 2020
@rdeltour rdeltour added the bug label Mar 3, 2020
@pkra
Copy link
Contributor Author

pkra commented Mar 3, 2020 •
edited
Loading

Thanks @rdeltour. Looks like

ace/packages/ace-core/src/scripts/ace-extraction.js

Line 130 in 896d1da

let hxElems = document.querySelectorAll('h1, h2, h3, h4, h5, h6');

is too simple. I'm happy to try making a pull request.

@rdeltour
Copy link
Member

rdeltour commented Mar 3, 2020

Yes, I had a quick look at the code and came to the same conclusion. PR welcome of course!

@laudrain
Copy link

laudrain commented Mar 3, 2020 •
edited
Loading

@pkra I understand the technical issue.
But i'm wondering why not use a <h1> tag in the first place instead of a <div>?

@pkra
Copy link
Contributor Author

pkra commented Mar 3, 2020 •
edited
Loading

@laudrain this was just a minimal example to exhibit the bug with minimal side effects.

Of course, the First Rule of ARIA applies but sometimes authors cannot use the native HTML element and sometimes there is no native element (e.g., there is no h7 and above).

@laudrain
Copy link

laudrain commented Mar 3, 2020

@pkra understood.

pkra added a commit to pkra/ace that referenced this issue Mar 3, 2020
@pkra
fix: detect role=heading for outline
9831cb2 
Enables ace-extraction's getHeadings() to detect headings marked
via role=heading; uses aria-level or the spec fallback.

Fixes daisy#308
@pkra pkra mentioned this issue Mar 3, 2020
Merged
pkra added a commit to pkra/ace that referenced this issue Mar 3, 2020
@pkra
fix: detect role=heading for outline
33c27d8 
Enables ace-extraction's getHeadings() to detect headings marked
via role=heading; uses aria-level or the spec fallback.

Fixes daisy#308
danielweck pushed a commit that referenced this issue Apr 7, 2020
@pkra
fix: enable role=heading for HTML outline (PR #309 Fixes #308)
4647ebb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rdeltour rdeltour

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: detect role=heading for outline pkra/ace
3 participants
@pkra @rdeltour @laudrain