Lint compiled HTML

[domoscargin]

We currently have no built-in way of ensuring our compiled HTML changes in the way we expect, and doesn't break.

This PR adds HTML linting using html-validate. There are quite a few commits, but they're all really small, promise!

There are some issues to consider:

• I've ignored a coupla "WCAG techniques" type rules which we might want to review as part of our accessibility work (495be15 and d1dea99)
• I've tried to avoid inline validation as far as possible, but sometimes this was the most targetted way
• We currently have status labels contained within <h1> tags, which introduces invalid child elements. See Status tags in headings are invalid HTML #2606
• I've popped in a temporary inline fix for Nunjucks parameters with the same name produce duplicate captions/IDs #2260 until that's fixed
• I've allowed nested forms for the few cases where that's an issue: Some pages have nested <form> elements #2609. Chrome, at least, simply rips out the inner <form> elements, but it's still messy

[netlify]

✅ You can preview this change here:

Name	Link
🔨 Latest commit	ffd0391
🔍 Latest deploy log	https://app.netlify.com/sites/govuk-design-system-preview/deploys/63f6270dfceebc0008281b0f
😎 Deploy Preview	https://deploy-preview-2569--govuk-design-system-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[querkmachine]

Do you have a sample of what problems it's finding? Nearly 2000 issues seems like an excessive number!

Also I imagine there are examples of invalid code where things are wrong-on-purpose because they have better outcomes for the end user (e.g. our use of aria-attributes on conditional checkboxes/radios). We probably want to treat those exceptionally.

[domoscargin]

I swiped the config for this from the govuk-frontend component tests, so hopefully most of the special cases are already accounted for, but I haven't delved too deeply yet.

Rules broken:

https://html-validate.org/rules/void-style.html
https://html-validate.org/rules/wcag/h30.html
https://html-validate.org/rules/aria-label-misuse.html
https://html-validate.org/rules/attr-quotes.html
https://html-validate.org/rules/no-implicit-close.html
https://html-validate.org/rules/element-permitted-content.html
https://html-validate.org/rules/attr-case.html
https://html-validate.org/rules/no-deprecated-attr.html
https://html-validate.org/rules/no-dup-id.html
https://html-validate.org/rules/attribute-allowed-values.html
https://html-validate.org/rules/wcag/h32.html
https://html-validate.org/rules/long-title.html
https://html-validate.org/rules/attr-delimiter.html
https://html-validate.org/rules/wcag/h63.html
https://html-validate.org/rules/form-dup-name.html
https://html-validate.org/rules/close-order.html

There's a lot of what I'm pretty sure is noise:

• aria-label-misuse because we've got aria-labels on our app-navigation__subnav ul elements, which I assume is a deliberate choice.
• void-style errors mostly seem annoyed about us self-closing <link> elements which feels heavy-handed.
• wcag/h30 on our top anchor: <a id="top"></a>

Probably legit errors include:

• Most of the attr-quotes look like they could be switched to double quotes with no issue.
• no-implicit-close: there's a few unclosed <p> tags floating about: <p>Share your experience, research and skills to help us reiterate the Task list pages pattern into a component.<p>

🤷

• element-permitted-content: Doesn't like the "experimental" div contained in the h1 on the Resources and Tools page
• The frameBorder attribute is the main culprit: causing both attr-case and no-deprecated-attr errors (about 530 errors in total)

Etc, etc.

So I'm assuming 90% of the errors could (safely) be ignored, but haven't really had a close look.

[colinrotherham]

@domoscargin I think this it great 🙌

Gives us more confidence that Markdown/Nunjucks is outputting what we want

[domoscargin]

I've created an issue to deal with some invalid HTML here: #2606

Down to 65 errors to check.

[domoscargin]

I've added another issue to deal with nested form elements here: #2609

In addition, #2260 is causing form-dup-name errors. This wouldn't happen if the rule checked for unique values, as suggested by @colinrotherham: alphagov/govuk-frontend#3270 (review), but I feel like at a certain point, the rule becomes a bit toothless if we're basically ignoring all its results, so better to fix the cause.

Once #2606, #2609 and #2260 are fixed, we can remove some of the changes made here.

[colinrotherham]

Have we got an issue for this one?

[domoscargin]

It was a deliberate choice for accessibility: 4ff06e4

@36degrees dya reckon I should make an issue to review this?

[36degrees]

I'm not sure where the list of 'allowed elements' on https://html-validate.org/rules/aria-label-misuse.html comes from… according to https://www.w3.org/TR/html-aria/#el-li li is not listed as 'Naming Prohibited' and the listitem role inherits the aria-label property… 🤔

The rationale in the linked commit makes sense to me, but I think given it's been flagged it might be worth asking @davidc-gds to review it just in case? Whether or not we need an issue for that I'm unsure – maybe ask David?

[domoscargin]

Interesting. Might dig about a bit in the html-validate source code and figure out if it's worth filing a bug. Will speak to David as well.

[domoscargin]

Ah, so the <ul> text reads:

Global aria-* attributes and any aria-* attributes applicable to the allowed roles.

Which is sort of ambiguous, but I think basically means we need to apply a role to the <ul> element - if we do that, then html-validate is happy.

By default, the <ul> has the list role, which is:

allowed, but NOT RECOMMENDED.

I guess the menu role or navigation feels most appropriate here? Though it's already contained within a nav.

[domoscargin]

Basically, I think this could do with an issue 😂

I'll make one

[36degrees]

Aah, of course – not sure why I was looking at <li>!

By default, the <ul> has the list role, which is:

allowed, but NOT RECOMMENDED.

I believe when they say it's 'allowed but not recommended' they mean it's not recommended to set this role explicitly – if you take a look at the headings for the table it says "Implicit ARIA semantics (explicitly assigning these in markup is NOT RECOMMENDED)".

I guess the menu role or navigation feels most appropriate here? Though it's already contained within a nav.

We should avoid the navigation role for exactly that reason – it's already in a nav region. The menu role is designed for interfaces that mimic desktop app style interfaces where you have menus with lists of items, like in a 'menu bar' and wouldn't be appropriate here.

list still feels like the appropriate role – I think if we changed it, we'd lose the information like 'List, 10 items' that screen reader users get which helps them understand how many links there are, for example.

Still worth having an issue to explore further, but I think either 1) having an aria-label is fine on a <ul> and is useful for users or 2) we should just remove the aria-label. I don't think changing the role makes sense.

[domoscargin]

I've added an issue and linked to this discussion.

#2624