Update HTML validator image and fix newly found issues #3479
Conversation
react-table (at least the versions before v8, which is a complete rewrite we don't want to deal with yet) add role attributes for ARIA purposes even when they are completely useless and actually fail HTML validation. See TanStack/table#2862 This implements the suggestion in the last comment on that PR, actively dropping any role that react-table might add. In our usage, these should always be useless AFAICT since our tables are generally actual tables.
reosarevok
force-pushed
the
update-html-validator
branch
from
0996610 to
b7ec0a1
Compare
|
@brainzbot, retest this please |
| @@ -77,7 +77,7 @@ export component MultipleSelectField( | |||
| return ( | |||
| <select {...selectProps}> | |||
| {allowEmpty | |||
| ? <option value="">{'\xA0'}</option> | |||
| ? <option label=" " value="">{'\xA0'}</option> | |||
There was a problem hiding this comment.
It actually might be better to add this to the ignored list in lib/MusicBrainz/Server/Test/HTML5.pm. Because label=" " doesn't actually do anything and isn't required for the site to work. Ignoring the error will at least continue to log it so that we might not entirely forget to fix it later. What do you think?
There was a problem hiding this comment.
Actually, I saw this doesn't currently make automated tests fail, although that seems to be because filters never get tested and elsewhere where it does get tested the nbsp is encoded differently, so mostly just luck. It was failing for me when I tried to test the page source by hand in the browser, and it is clearly an issue, but I guess that means we don't need to change anything right now - opened MBS-13944 for improving the situation in the future.
| </li> | ||
| <li className="separator" key="separate-next" /> | ||
|
|
||
| {nextPage == null ? ( |
There was a problem hiding this comment.
I'm not sure if this is even valid (probably not really?) but it doesn't seem
to have any purpose in any case and it triggers validation tests about the
two closing /li in a row.
Nested <li> elements are definitely invalid, so the "two closing /li in a row" error sounds like the browser is adding an implied end tag for the first li when the second one appears. Because React doesn't allow you to have unbalanced/mismatched tags like that, outside of dangerouslySetInnerHTML.
When we changed this file several years ago, we forgot to do so that loading the form with jQuery will still set the cookie to keep the filter open next time. Before commit 49ab946 we always started from scratch after loading the form, so the cookie got set that way.
This was wrongly converted from the TT paginator, which just closes the separator li before opening the next. I'm not sure if this is even valid (probably not really?) but it doesn't seem to have any purpose in any case and it triggers validation tests about the two closing /li in a row.
This is needed for form rel attributes to be accepted as valid.
reosarevok
force-pushed
the
update-html-validator
branch
from
575e376 to
772aa99
Compare
Problem
Our existing HTML validator image is very old, and does not support
relforform, which I need for #3474Sadly, updating the validator brings with it a bunch of (mostly legit) new issues it finds, so we need to fix those too.
Solution
This fixes different issues found by the latest validator, until we hopefully pass all tests.
Testing