Problem/Motivation

The theme has several markup errors and warnings in the W3C Validator . Many elements have two identifiers or unnecessary role attributes.

Proposed resolution

I fixed several of these errors and warnings in the attached patch by tweaking the twig templates. At first sight it does not look it causes rendering issues (tested on Linux/Firefox, Linux/Chrome, Windows/Edge, and Windows/IE).

Remaining tasks

Not all errors and warnings are fixed by this patch. I guess some of the erroneous html is produced by Drupal core. Maybe someone more experienced can take a look?

At least these bugs I was not able to fix:

  • header element is containing the unnecessary banner role (warning)
  • article element is containing the unnecessary article role (warning)
  • footer element is containing the unnecessary contentinfo role (warning)
  • form element is containing the unnecessary form role (warning)
  • Attribute aria-required is unnecessary for elements that have attribute required (warning)
  • search field input element is containing a size attribute with a bad value (error)
  • duplicate ID edit-actions on contact form page (error)
  • duplicate ID edit-submit on contact form page (error)
CommentFileSizeAuthor
#19 w3c-responsive-menu-duplicate_id.patch884 bytessd123
#19 w3c-pager.patch816 bytessd123
#15 w3c-varia.patch4.59 KBsd123
#15 w3c-duplicate-id.patch4.2 KBsd123
#15 w3c-roles.patch6.93 KBsd123
#13 AT 2.0 contact form html.txt7.19 KBslayer722
w3c.patch15.61 KBsd123

Comments

sd123 created an issue. See original summary.

mattbloomfield’s picture

mattbloomfield commented

I just committed a new version, with a lot of those fixed. I also added some others. The big exception is the logo dimensions, which I'm still working on and hope to find a solution through the UI at some point.

sd123’s picture

sd123 commented

You forgot to apply a few changes:

  1. block/block--responsive-menu.html.twig: unnecessary role="navigation" removed in the nav tag
  2. block/block--system-branding-block.html.twig: removed itemprop="url" twice
mattbloomfield’s picture

mattbloomfield commented

I just reverted them and I have to look over some more things. These changes interfered with search api and made it redirect to the homepage.

sd123’s picture

sd123 commented

What is broken exactly? It looks to be working fine on my website.

mattbloomfield’s picture

mattbloomfield commented

In particular, Search API. The form redirected to the homepage instead of going to the search page. You wouldn't notice it unless you're using Search API instead of the default search block. I'll have to figure out how that's happening and make adjustments.

slayer722’s picture

slayer722 commented

Hello,

I can confirm that the 1.4.13 release make a lot of troubles.
I used it associated with Pixture Reloaded on a Drupal 9.1.5 website.
With me it was impossible to submit any forms, even to just login or send a message with contact form.
Absolutely no errors was shown on the screen or in logs, the page was just like refreshed.
What a headache to understand that the cause was just the theme update.
Anyway thanks for the effort on this issue.

I hope you'll can solve this.

Regards

mattbloomfield’s picture

mattbloomfield commented

Thanks for understanding. It's a little more complex than I initially thought. But the commit from a few days ago reverted things, so you can use that and I'll work on getting this right and into a release in the hopefully near future.

mattbloomfield’s picture

mattbloomfield commented

Also, would it help to have a d9-compatible version of Pixture Reloaded so you can use it with composer?

sd123’s picture

sd123 commented

Is it related to the removal of the role attributes or of the duplicate identifiers? In case it is the latter, maybe it can be tested to remove the other identifier?

mattbloomfield’s picture

mattbloomfield commented

It could be a number of things, but I think your idea is worthwhile and I'm going to test out as many possibilities I can so that this can get into a release.

slayer722’s picture

slayer722 commented
StatusFileSize
new AT 2.0 contact form html.txt7.19 KB

I don't use Composer a lot, I only manage 3 small websites, for me it's not very important thanks.
So far I was able to manually install and use Pixture Reloaded with Drupal 9.1 without problems, I just had to modify "core_version_requirement" in info.yml
Of course it would be great to see Pixture Reloaded updated, it’s the same author, I hope he’s okay and will give news someday.

I explored the problem, at my level, please take a look at the attached file, I compared the HTML output of the contact form between 1.4.13 and .14.

Almost all parameters are missing in the opening of the form, missing action, method, etc.

Thank you for you work, I'm happy to be able to still using AT with D9.

mattbloomfield’s picture

mattbloomfield commented

I'd also like to include logo height and width dimensions through the UI interface at some point. If someone wants to make a patch, they're more than welcome.

sd123’s picture

sd123 commented
StatusFileSize
new w3c-roles.patch6.93 KB
new w3c-duplicate-id.patch4.2 KB
new w3c-varia.patch4.59 KB

I created three patches.

  1. w3c-duplicate-id.patch removes duplicate "unique" identifiers and it looks like this patch causes no harm on my test site.
  2. w3c-varia.patch cleans up the button code and removes the itemprop attributes and it also does not look to cause any harm.
  3. w3c-roles.patch removes the role attributes and this breaks the site.

Probably it is safe to apply the first 2 patches, but better check. Note that I tested this on my test D9 site. I have no D8 or D9 production site as I am migrating from D7.

The problem thus seems either the theme or Drupal core relies on the existence of "unnecessary" roles. I don't know how the code works but maybe it can be fixed by relying on a data-* attribute instead of a role?

mattbloomfield’s picture

mattbloomfield commented

Thanks a ton for making these patches. I'm going to test them within the next few days to a week to see if it interferes with anything like search API

  • mattbloomfield committed 8fb608f on 1.0.x 
    Issue #3203393 by sd123, slayer722, mattbloomfield: Fix W3C markup...
mattbloomfield’s picture

mattbloomfield commented

I tested the patches and committed them to the dev version. I'll wait a bit before creating a new release, just to be sure. If anyone has any issues, please let me know ASAP.

sd123’s picture

sd123 commented
StatusFileSize
new w3c-pager.patch816 bytes
new w3c-responsive-menu-duplicate_id.patch884 bytes

It seems to work fine on my test site, but maybe it is best to have someone else try as well. Btw, why is there no dev release that can be installed using composer?

I also discovered 2 other W3C compliancy bugs.

The pager code contains 2 nest ul tags. I replaced the first with div. Chances are low this patch breaks things.

The responsive menu has a duplicate id. It does not look to cause any harm on my test site.

sd123’s picture

sd123 commented

I saw the last 2 patches were not applied in the new release. Is there any reason for that?

PS: the pager patch is required on my site to receive a 100% accessibility score in the Chrome Lighthouse testing tool.

  • mattbloomfield committed de66165 on 1.0.x 
    Issue #3203393 by sd123, slayer722, mattbloomfield: Fix W3C markup...
mattbloomfield’s picture

mattbloomfield commented

Sorry about that, I must have overlooked it. But I just committed it to the dev version and it'll make its way into the next release.

mattbloomfield’s picture

mattbloomfield commented
Status: Active » Fixed
sd123’s picture

sd123 commented

What about a solution for the w3c-roles.patch that causes issues?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.