w3 validator warnings

[dennmuel]

Hi @NateWr,

we ran our ojs instance through the w3 validator (site page and journal page) and got some errors and warnings. It's mostly unncessary attributes or bad attribute values, however I'm not sure, if they are not indeed needed for screenreaders and the like. Same goes for the aria errors.

Examples: Bad value content for attribute role on element div. The navigation role is unnecessary for element nav. The aria-controls attribute must point to an element in the same document.

I'm sure you know a lot more about this, so I'd be grateful for some context about these warnings and errors. EDIT: I've opened a PR #131 proposing some changes.

Best regards Dennis

[NateWr]

Thanks @dennmuel! Ok, let me run through these:

Attribute aria-labelled-by not allowed on element nav at this point." Interesting! Ok, what we need to do is use aria-label instead. So this:

<nav id="accessibility-nav" class="sr-only" role="navigation" aria-labelled-by="accessible-menu-label">
  <div id="accessible-menu-label">
    {translate|escape key="plugins.themes.bootstrap3.accessible_menu.label"}
  </div>

Should be this:

<nav id="accessibility-nav" class="sr-only" role="navigation" aria-label="{translate|escape key="plugins.themes.bootstrap3.accessible_menu.label"}">

The XXXX role is unnecessary for element XXXX Although semantically accurate, we leave the roles there because, as far as I'm aware, some older screen readers do not recognize landmark tags (<header>, <nav>, etc).

An img element must have an alt attribute This is a content issue. You should be able to add an alt tag to each one. I notice other journal thumbnails on your site have one.

Bad value xx_XX for attribute lang on element This is related to markup from the language block. I've filed an issue here: https://github.com/pkp/pkp-lib/issues/5015

The type attribute is unnecessary for JavaScript resources Although unnecessary I'm happy to leave this in for clarity and out of inertia.

The aria-controls attribute must point to an element in the same document. Oh! In header.tpl, the following:

<button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#nav-menu" aria-expanded="false" aria-controls="navbar">

Should be:

<button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#nav-menu" aria-expanded="false" aria-controls="nav-menu">

Bad value content for attribute role on element Yes, the role="content" attribute should be removed from indexJournal.tpl.

Element p not allowed as child of element a in this context I think this is incorrect. My understanding is that HTML5 now allows this: https://davidwalsh.name/html5-elements-links

Thanks for the review and for putting your site up for inspection! Would you mind updating your PR to reflect these changes?

[dennmuel]

Hi @NateWr,

thanks for your quick reply.

An img element must have an alt attribute This is a content issue. You should be able to add an alt tag to each one. I notice other journal thumbnails on your site have one.

Yes, I thought so. However when I uploaded that image, it didn't save the alt text I provided. Maybe that problem is related to localization since the journal in question is English only, while the rest is German by default and English additionally. Maybe I can investigate further later on (although that might have to wait until after my vacation). Edit: Nevermind, worked now, dunno where I erred.

Bad value xx_XX for attribute lang on element This is related to markup from the language block. I've filed an issue here: pkp/pkp-lib#5015

Thanks :)

The type attribute is unnecessary for JavaScript resources Although unnecessary I'm happy to leave this in for clarity and out of inertia.

Okay, that renders my PR over at pkp-lib obsolete. I'll close it then. Edit: Too slow, you already did ;)

The aria-controls attribute must point to an element in the same document.

Should already be done in the PR.

Bad value content for attribute role on element Yes, the role="content" attribute should be removed from indexJournal.tpl.

Would role="main" be sufficient instead?

Element p not allowed as child of element a in this context I think this is incorrect. My understanding is that HTML5 now allows this: https://davidwalsh.name/html5-elements-links

Okay, let's leave this one, then.

Thanks for the review and for putting your site up for inspection! Would you mind updating your PR to reflect these changes?

Already on the way :)

[NateWr]

Would role="main" be sufficient instead?

No, let's just remove the role. We apply role="main" to the <main> tag appropriately. Not sure where this extra role came from.

[dennmuel]

Okay, I updated the PR accordingly.

[NateWr]

Thanks @dennmuel!

[dennmuel]

Thank you, too @NateWr!