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

role="doc-pagebreak" rejected on a element with no href attribute #1022

Closed
tofi86 opened this issue Mar 25, 2019 · 17 comments
Closed

role="doc-pagebreak" rejected on a element with no href attribute #1022

tofi86 opened this issue Mar 25, 2019 · 17 comments
Assignees
Labels
status: has PR The issue is being processed in a pull request
Milestone

Comments

@tofi86
Copy link
Collaborator

tofi86 commented Mar 25, 2019

A pagina EPUB-Check user is reporting the following issue with 4.2.0-rc:

HTML:

<a role="doc-pagebreak" id="page6" title="6" epub:type="pagebreak" class="pageref">6</a>

EPUBCheck report:

ERROR(RSC-005): ch0001.xhtml(11,88): Error while parsing file: value of attribute "role" is invalid; must be equal to "button", "checkbox", "doc-backlink", "doc-biblioref", "doc-glossref", "doc-noteref", "link", "menuitem", "menuitemcheckbox", "menuitemradio", "option", "radio", "switch", "tab" or "treeitem"

According to https://idpf.github.io/epub-guides/epub-aria-authoring/ should be tagged with <hr> but:

  • In addition to the specific elements listed in the table, the following elements accept any role value.

a (without an href attribute) [...]

What's wrong here?

@rdeltour
Copy link
Member

This seems to be a bug in the schemas, as a elements without a href should accept any role indeed (as defined in ARIA in HTML).

I'll look into it!

@rdeltour rdeltour self-assigned this Mar 25, 2019
@rdeltour rdeltour added priority: high To be processed and published in the next release type: bug The issue describes a bug status: accepted Ready to be further processed labels Mar 25, 2019
@rdeltour rdeltour changed the title 4.2.0-rc: role="doc-pagebreak" not valid role="doc-pagebreak" rejected on a element with no href attribute Mar 25, 2019
@rdeltour
Copy link
Member

In the meantime, I'd recommend using a neutral element like a span, or an hr separator, as they probably are a better fit to represent page breaks (in HTML, an a element with no href attribute "represents a placeholder for where a link might otherwise have been placed").

@rdeltour
Copy link
Member

This seems to be a bug in the schemas

Apparently this was reported already as validator/validator#638, which is marked as wontfix. Quoting the issue here for convenience:

Making different role values allowed for the a element based on whether or not it has an href attribute adds additional complexity to the checker sources beyond what I’m willing to support.

So I’m marking this one wontfix.

If anybody cares enough about this to take the time to write up a patch to support, I’d be willing to review but still won’t guarantee I’d actually land it — because at this point I can’t imagine any actual use cases that are compelling enough to merit the additional complexity.

We now have to gauge whether this is problematic for EPUB and if we can reasonably provide a patch, or if we can just follow the lead and mark it as status: blocked and type: wontfix. At first sight I'm tempted to go for the latter option.

@rdeltour rdeltour added status: blocked Blocked by another issue or situation and removed priority: high To be processed and published in the next release status: accepted Ready to be further processed labels Mar 25, 2019
@mattgarrish
Copy link
Member

My inclination is to leave it as an error to apply non-link roles on a, too. As you've already pointed out, it's technically not good practice to use a elements except for links and what are essentially placeholder links when you omit href. Page breaks are destinations, which is the use of a that HTML5 wrote out.

@mattgarrish
Copy link
Member

Page breaks are destinations, which is the use of a that HTML5 wrote out.

And in case it needs saying, I also can't see this being the compelling use case that gets a patch accepted.

@rdeltour
Copy link
Member

Yeah I just submitted a PR to fix that in the Nu HTML Checker 😅 (evening fun).
I obviously agree with you previous comments, but this would need to be changed in the ARIA in HTML spec, not in validators.

Anyways, whether this might or might not be a compelling case has to do with how widespread the practice of using a-with-no-href is for marking up page breaks… Let's see if there's any reaction to the PR.

@rdeltour
Copy link
Member

Btw, using a with no href as anchor destinations is not uncommon as far as I know (going back to early HTML), which is likely why ARIA in HTML is making a special case. As usual it is difficult to get data on how widespread it is in EPUB.

I asked on Twitter and letex folks said:

We do it all the time (for ex. <a epub:type="pagebreak" id="page_101"/>), with or without content, depending on whether they are merely targets for the page map or whether they contain renderable content (the page number).

@mattgarrish
Copy link
Member

Well, we shouldn't start pushing to legitimize the uses of epub:type... ;)

ARIA in HTML isn't a recommendation at this point, though, so it'll be interesting to see how this plays out. We may not want to prematurely legitimize practices that won't survive.

@rdeltour
Copy link
Member

We may not want to prematurely legitimize practices that won't survive.

Now you make me feel bad about submitting that PR 😬😅

Joke aside, what we would really need is:

  • data on whether it's used already in production (in distributed EPUBs), and how much
  • tests with SR+RS to see if it has a (negative) impact on accessibility

@mattgarrish
Copy link
Member

mattgarrish commented Mar 26, 2019

Now you make me feel bad about submitting that PR

All I meant is we probably shouldn't rush to support this in epubcheck. If it doesn't make the coming release, so be it. Better to be late to the party than get it wrong.

  • data on whether it's used already in production

Given that we've always promoted pagebreaks through epub:type, I'd suspect very little. There's also no reason to rush people to roles at this point, but that's maybe something we should reconsider in Ace. We probably should tone down the messaging that every epub:type needs a role, and reconsider the elements we should even be considering equivalency on.

@laudrain
Copy link
Collaborator

Good practices in EPUB world have been specified as:
<span epub:type="pagebreak" id="FOLIO-25" title="25" />
(from EDUPUB for instance)
or
<span xml:id="page361" epub:type="pagebreak" title="361"/>
(in a book titled "EPUB 3 Bets Practices" written by Markus Gylling and Matt Garrish).

IMO, using an a tag is a bad practice...

@garconvacher
Copy link

<a role="doc-pagebreak" id="page6" title="6" epub:type="pagebreak" class="pageref">6</a>
tests with SR+RS to see if it has a (negative) impact on accessibility

SR users can browse from link to link. a with pagebreak can be useful (or not).

Tests with 2 pagebreak on same page:

  • NVDA:
    • Readium 1, ignore the navigation by links but say "clickable, separator, 6" on focus. Same with Readium 2 (desktop) but no "separator".
    • ADE ignore the links and their contents.
  • VoiceOver:
    • Lisa ignore the links and their contents (iOS 11/12).
    • Books (iOS 12) is a mess (as usual): navigation by links only find the first a but pronounce the content of all links.
    • iBooks (iOS 11) ignore the links and their contents.

Note: <span id="pageX" title="X" role="doc-pagebreak" epub:type="pagebreak"></span> don't work with VoiceOver nor NVDA except with Readium 1 ("X, separator, X").

@tofi86
Copy link
Collaborator Author

tofi86 commented Mar 26, 2019

As usual it is difficult to get data on how widespread it is in EPUB.

The original reporter stated, that they were always using a elements for pagebreak tagging and it would be troublesome to replace all that with spans in backlist and new EPUB's. They would rather remove the role attribute than change to span.

@mattgarrish
Copy link
Member

The original reporter stated, that they were always using a elements for pagebreak tagging and it would be troublesome to replace all that with spans in backlist and new EPUB's.

There also isn't a rush to changeover. For the most part, they're just destinations to jump to, with the possibility to provide visual indicators in reading systems (though I don't think many actually do). The page list has always been the more important piece.

The announcing of page breaks is partly predicated on the ability to turn off such announcements, as they are disruptive to immersive reading. Making them ARIA roles effectively turns them on to be announced (when AT do begin supporting DPUB roles), but doesn't leave much control for turning them off.

@clapierre
Copy link

FYI: So currently on the DAISY KB there is the following:

Can I use a tags for page numbers?
The a element has two specific uses defined in HTML5: for links when the href attribute is present, and for placeholder links when it is not (e.g., a link that might be active in another context or after some interaction by the user). As page breaks are not links, and are never intended to be activated as links, it is incorrect to use them for page breaks markers.

Does this need to be adjusted?

@rdeltour
Copy link
Member

Does this need to be adjusted?

No, the KB is on point: a elements are not intended to represent anything other than links, so they're not appropriate for marking up page breaks.

That ARIA in HTML allow any role on a elements with no href attribute does not change that; it's just that technically it's not an error to do it, it's just a bad practice.

@rdeltour
Copy link
Member

My PR validator/validator#774 has been merged, so the Nu HTML Checker is now aligned with the spec and will no longer reject the doc-pagebreak role on a elements with no href. Again, doing it is considered bad practice, but it's not the role of conformance checkers to override the specs or report bad-but-conforming practices; this is better left out to best practice guidelines (like the KB).

I'll create a PR to port the schema fix to EPUBCheck.

rdeltour added a commit that referenced this issue Mar 26, 2019
@rdeltour rdeltour added status: has PR The issue is being processed in a pull request and removed status: blocked Blocked by another issue or situation type: bug The issue describes a bug labels Mar 26, 2019
@rdeltour rdeltour added this to the 4.2.0 milestone Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: has PR The issue is being processed in a pull request
Projects
None yet
Development

No branches or pull requests

6 participants