fix: detect role=heading for outline

[pkra]

Enables ace-extraction's getHeadings() to detect headings marked
via role=heading; uses aria-level or the spec fallback.

Fixes #308

[pkra]

@rdeltour is this roughly the right direction?

[rdeltour]

@rdeltour is this roughly the right direction?

yes! 👍

[pkra]

Thanks, @rdeltour. Should I check aria-level values more carefully (e.g., non-negative) or will they be checked elsewhere anyway?

[rdeltour]

Should I check aria-level values more carefully (e.g., non-negative)

yes, I think all the values that are not conforming (0 or negative values) should be reported as heading level 2 (default value).

Ideally we should test this in browser+AT combinations to have a better idea how this is processed in real life, but until then sticking to the spec looks reasonable.

will they be checked elsewhere anyway?

Apparently it's not checked by Axe (see dequelabs/axe-core#1288). I don't think it's checked by EPUBCheck (to be verified).

[pkra]

@rdeltour I added a check for positive integers (also avoiding +'Infinity' becoming Infinity).

[pkra]

@rdeltour will you let me know if you need anything else from me?

[rdeltour]

@rdeltour will you let me know if you need anything else from me?

no, it all looks good 👍. I'm just a little overloaded these days 😅 I'll do a final review and merge it ASAP!

[pkra]

Thanks @rdeltour. No rush from my end, I just wanted to check before I lose track.

[rdeltour]

@danielweck can this be included in the upcoming 1.2.0 version?

[danielweck]

I propose that we merge this PR into the master branch, as originally intended. This may or may not be followed by an official release update (version 1.1.2, see https://github.com/daisy/ace/releases ) ... to be discussed.

Then I will (as usual) "backport" codebase changes from master into the ace-next branch (version 1.2.x beta). I am not sure about code conflicts yet, let's see...

[danielweck]

The master unit tests pass: https://travis-ci.org/github/daisy/ace/builds/672176684

[danielweck]

The ace-next unit tests pass too: https://travis-ci.org/github/daisy/ace/builds/672176804

[danielweck]

Thank you very much @pkra your contribution is much appreciated.

Note that the ace-next branch (Ace 1.2 beta) contains many updated NPM packages, including the Axe dependency. I will likely soon publish an NPM update version 1.2.0-beta.7 to the next package tag ( https://www.npmjs.com/package/@daisy/ace ), including this PR fix.

The ace-next branch is used to build the Ace desktop app ( https://github.com/daisy/ace-gui ) for which a release update is expected soon. The next version of Ace App will therefore include this PR fix too.