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

ARIA attributes should be allowed on SVG elements #846

Closed
brianagina opened this issue May 15, 2018 · 16 comments
Closed

ARIA attributes should be allowed on SVG elements #846

brianagina opened this issue May 15, 2018 · 16 comments
Assignees
Labels
status: completed Work completed, can be closed type: bug The issue describes a bug
Milestone

Comments

@brianagina
Copy link

I have been working on making an epub file accessible. Finally, the DAISY ACE showed no violations arising from the epub file after validation. On validating via the epubcheck 4.0.2 , I got 3 errors arising from parsing the epub file. The errors were as a result of making an image accessible:

1. Error while parsing file 'attribute "aria-label" not allowed here
2. Error while parsing file 'attribute "role" not allowed here, and
3. Error while parsing file 'attribute "alt" not allowed here

How can I solve this?

@brianagina
Copy link
Author

brianagina commented May 15, 2018

The following code is from an xhtml file generating the error. The errors are arising from 3 attributes used in the img element.
<svg xmlns="http://www.w3.org/2000/svg" height="100%" version="1.1" viewBox="0 0 500 857" width="100%" xmlns:xlink="http://www.w3.org/1999/xlink"> <image height="857" width="500" xlink:href="../images/cover.jpg" role="doc-cover" aria-label="Cover" alt="A boy called Pat and a girl called Mata heading to school."/> </svg>

@tofi86 tofi86 reopened this May 15, 2018
@tofi86 tofi86 added the status: needs review Needs to be reviewed by a team member before further processing label May 15, 2018
@JayPanoz
Copy link

@tofi86 Not sure about that but it may well be SVG 1.1 (the one we use in the EPUB spec) versus SVG 2.0. Maybe that can help with the review.

Aria attributes were added in SVG 2.0, they did not appear in SVG 1.1. As for alt, an error is expected since you should use <title> (see SVG Graphics).

Note 1.1 is somewhat painful in some cases e.g. custom data-* attribute, which works without any issue but will throw an error in ePubCheck as it is strictly speaking SVG 2.0 (so you must create a namespace etc.). That being said, “SVG2, it’s complicated.”

@rdeltour
Copy link
Member

The alt attribute is not allowed on SVG image elements, but since SVG 2 ARIA role and aria-* (state and properties) attributes should be allowed indeed.

I could reproduce that this is still reported as an error on v4.2.0-beta.

@rdeltour rdeltour added status: ready for implem The issue is ready to be implemented and removed status: needs review Needs to be reviewed by a team member before further processing labels Feb 26, 2019
@rdeltour rdeltour added this to the 4.2.0-RC milestone Feb 26, 2019
@rdeltour rdeltour changed the title epub file validated by DAISY ACE throwing error on validation by epubcheck ARIA attributes should be allowed on SVG elements Feb 26, 2019
@rdeltour
Copy link
Member

I added a test case in branch fix/846/svg-aria.

@mattgarrish can you please confirm that ARIA attributes should indeed be supported? (in light of w3c/epub-specs#1219)

@JayPanoz
Copy link

FWIW, as someone who spent an entire workday adding – do the right a11y thing – then removing ARIA attributes on hundreds of SVGs a few years ago, I’m highly in favour of supporting them (and there are so many how-tos using those irl that it’s kinda frustrating to discover they will create errors once it’s done).

@mattgarrish
Copy link
Member

Right, let's not take the attributes out and get into a game of modifying the official schemas. The end resolution was that we'll sort of pretend we're referencing 1.1 for spec purity but the reality is we'll actually allow whatever the browsers are now accepting, which as @dauwhe pointed out in w3c/epub-specs#1219 (comment) is closer to SVG2.

@rdeltour
Copy link
Member

Right, let's not take the attributes out and get into a game of modifying the official schemas.

Just for clarifications: these attributes seem to be rejected by the new schemas in 4.2.0-beta. That is likely due to how we integrate these when validating as SVG-only, I'll have a deeper look.

@rdeltour rdeltour added the type: bug The issue describes a bug label Feb 26, 2019
@rdeltour rdeltour self-assigned this Feb 26, 2019
@mattgarrish
Copy link
Member

Seems to be working for me. There are elements that the attributes aren't allowed on, but I haven't seen them rejected.

@rdeltour
Copy link
Member

Have you tried the test case in branch fix/846/svg-aria?

@mattgarrish
Copy link
Member

No, I was using the sous le vent book from the samples and applying roles and attributes to various elements without problem. It must be a localized bug.

@mattgarrish
Copy link
Member

It doesn't report role as an error, only aria-label.

Looking at the schema, wouldn't the ? at the end here be the culprit:

(common.attrs.aria.implicit.img | common.attrs.aria)?,

@mattgarrish
Copy link
Member

Sorry, that's from attlist.image in svg-image.rnc in case it's not obvious.

@rdeltour
Copy link
Member

Looking at the schema, wouldn't the ? at the end here be the culprit

You mean it should be * instead right? Possibly. I'm surprised however this hasn't been raised already and fixed in validator.nu. The pattern seems to be used in various other elements.

@rdeltour
Copy link
Member

For ref, here's the validator.nu commit which introduced it.

@mattgarrish
Copy link
Member

I was thinking it was unnecessary, but that's probably not the issue after running a couple of tests. It also validates fine in the online validator, which presumably is using these same schemas. I'll keep sniffing around and see if I can make any sense of what is happening.

@mattgarrish
Copy link
Member

mattgarrish commented Feb 26, 2019

For the record, we were missing this line from nu validator driver file:

SVG.Core.attrib &= aria.global?

So only the attributes defined in common.attrs.aria were allowed, which is why we both seemed to be right in what we were checking. :)

rdeltour added a commit that referenced this issue Feb 26, 2019
Add `aria.global` attributes to `SVG.Core.attrib`, in both SVG and XHTML
integration schemas.

Fix #846
@rdeltour rdeltour added status: completed Work completed, can be closed and removed status: ready for implem The issue is ready to be implemented labels Feb 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: completed Work completed, can be closed type: bug The issue describes a bug
Projects
None yet
Development

No branches or pull requests

5 participants