vaadin-icon-set custom element in page head breaks html parsers

rolfsmeds commented 1 year ago

Description

The vaadin-icon-set custom element injected into the page <head> by vaadin-icon breaks W3C html validators https://validator.w3.org/ and https://validator.w3.org/nu/about.html#extras so that they are unable to parse the contents of the page's <body> and instead fail with the following error:

Error: Start tag body seen but an element of the same type was already open.

(Based on the mdn page for the <head> element, it might indeed be that custom elements are not permitted inside the element.)

This gives the unfortunate (and incorrect) impression that Vaadin applications contain severely invalid html. As the above validators are recommended to be used as part of accessibility validation procedures (e.g. BITV in Germany), this will likely to lead to bad impressions of Vaadin as a platform.

Expected outcome

These popular html validators should be able to parse a Vaadin-based UI.

Minimal reproducible example

Go to https://start.vaadin.com/app Copy the outerhtml of the html element in the iframe to https://validator.w3.org/#validate_by_input and click Check.

Steps to reproduce

See above.

Environment

Vaadin version(s): V23.2

Browsers

Issue is not browser related

web-padawan commented 1 year ago

Marked as enhancement since fixing this would require adding new API - see the prototype: https://github.com/vaadin/web-components/commit/559d27d3c2a5bf4d58538a1e8a1fba2e39b13c17

jouni commented 1 year ago

Can't the element be placed in the <body>?

web-padawan commented 1 year ago

Yes, we could try that too. But in general, I would prefer to have a solution not requiring to add it to DOM. This has caused problems with id for icons in the past, especially when we used iron-iconset-svg.

knoobie commented 1 year ago

Note: In my current application there are two vaadin-iconset in the head and also an iron-iconset-svg even tho I'm not using any legacy icons - are some components still using iron-icons? And potentially fixed by https://github.com/vaadin/web-components/pull/4879 in V24?

Setup: Vaadin 23.1.15 / no add-ons / FontAwesome (font icons) and cleave.js as additional JS deps

Edit: At least the behaviour that two vaadin-iconset are present can be easily reproduced by just adding sayHello.setIcon(VaadinIcon.SEARCH.create()); to a project created from start.vaadin.com in the default view and running in production mode (development is probably the same)

web-padawan commented 1 year ago

Yes, in V24 there will be no longer iron-iconset-svg in the platform. It might be there in V23 especially in dev mode where all the components are imported (e.g. when using Vite, which is the default since 23.2, it gets included to the bundle).

knoobie commented 1 year ago

FYI: The WCAG WG has decided to drop the infamous 4.1.1 criteria in 2.2 🥳 https://www.w3.org/2022/11/22-ag-minutes.html#t08

knoobie commented 1 year ago

Workaround if somebody needs it:

Implement VaadinServiceInitListener
Add UIIinitListener with:

      ui.getUI().getElement().executeJs("setTimeout(function() {"
        + "  var iconsets = document.head.getElementsByTagName('vaadin-iconset');"
        + "  for (let i = 0; i < iconsets.length; i++) {"
        + "    document.head.removeChild(iconsets[i]);"
        + "  }"
        + "}, 0);");

(Of course, this is only valid if no vaadin icons are in use ;))

web-padawan commented 1 year ago

Thanks for reminding me about this issue, I'll create a PR soon to be included in Vaadin 24.

vaadin / web-components