No Code Attached Yet a11y
avatar brianteeman
brianteeman
5 Dec 2021

Pull Request #35610 by @nikosdion has broken the aria labelling of fields. This can easily be seen on every toolbar search button

Before

image

After

image

As you can see the value of aria-describedby must equal the value of the id

Any field using aria-describedby is therefore broken by this PR

This should be labelled as a release blocker

avatar brianteeman brianteeman - open - 5 Dec 2021
avatar joomla-cms-bot joomla-cms-bot - change - 5 Dec 2021
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 5 Dec 2021
avatar brianteeman brianteeman - change - 5 Dec 2021
The description was changed
avatar brianteeman brianteeman - edited - 5 Dec 2021
avatar chmst chmst - change - 5 Dec 2021
Labels Added: a11y
avatar chmst chmst - labeled - 5 Dec 2021
avatar chmst chmst - change - 5 Dec 2021
Labels Added: Release Blocker
avatar chmst chmst - labeled - 5 Dec 2021
avatar nikosdion
nikosdion - comment - 5 Dec 2021

I think you are making a false assertion there, Brian. I explicitly changed all of the form field layout files so that they use the actual $id (the intended id attribute) value passed by Joomla instead of $name (the field name attribute), fixing a long–standing bug in Joomla.

I did not change the searchtools layout files. They have the same bug I fixed. Namely, the layouts/joomla/searchtools/default.bar.php line 38 ERRONEOUSLY reads:

<div role="tooltip" id="<?php echo $filters['filter_search']->name . '-desc'; ?>">

instead of the correct

<div role="tooltip" id="<?php echo $filters['filter_search']->id . '-desc'; ?>">

According to Git Blame this is a change that you did yourself @brianteeman on May 14th, 2019 with PR #24892.

Claiming that I introduced a bug in Joomla when the simple matter of fact is that I actually fixed your bugs (just not all of them, apparently) is disingenuous. Please fix what you broke and keep in mind that the name property of a field object is meant to be used in the name attribute of the HTML element, whereas the id property of a field object is meant to be used in the id attribute of the HTML element, as it has been since at least Joomla 1.5 released in 2007.

avatar brianteeman
brianteeman - comment - 5 Dec 2021

The change that broke this is
image

Revert the change and you will see that the values for aria-describedby and id match

avatar brianteeman
brianteeman - comment - 5 Dec 2021

Re-reading what you wrote I believe you are confused between name= and aria-describedby=

avatar nikosdion
nikosdion - comment - 5 Dec 2021

No, Brian, between the two of us you are the only one who is confused.

I am fully aware that the aria-describedby HTML element attribute is meant to reference the id attribute of another element, reference: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-describedby_attribute I know how to peruse MDN, thank you very much.

The problem is how the label ID attribute was generated and how the aria-describedby attribute was generated. Both of these are things that you broke between two and half and three and a half years ago.

First, let's see what MDN has to say about the ID attribute. As per https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id

Note: Technically, in HTML5, the value for an id attribute may contain any character, except whitespace characters. However, to avoid inadvertent errors, only ASCII letters, digits, '_', and '-' should be used and the value for an id attribute should start with a letter. For example, . has a special meaning in CSS (it acts as a class selector). Unless you are careful to escape it in the CSS, it won't be recognised as part of the value of an id attribute. It is easy to forget to do this, resulting in bugs in your code that could be hard to detect.

For this reason, Joomla's Form package generates two DIFFERENT properties in the form object, called id and name. The id property is in the form of jform_field-name whereas the name property is in the form of jform[field-name]. They are different because they are meant to be used in a different context each, the outputted HTML element's id and name attributes respectively.

You have ERRONEOUSLY used name instead of id in TWO (2) Pull Requests you made yourself, #22165 from 3 ½ years ago and #24892 form 2 ½ years ago. Since you are using the field object's name property to construct the label's id attribute it is a glaring bug and I am surprised that anyone missed it and accepted your PR to begin with.

My PR #35610 has already fixed the bug you introduced more than three years ago in your PR #22165. The label element now uses the correct object property (id) to generate the label's id attribute.

I did not, however, touch the bug you introduced more than two years ago in your PR #24892 — I had not noticed it because this issue was introduced in a different PR than the first one that I fixed.

I already told you how the bug you introduced (and my PR uncovered) can be fixed. The only question is do you want me to submit a PR for it or would you rather do it yourself?

There is absolutely no confusion on my part about what is broken, why it's broken, who broke it and when it was broken. It was dead easy to figure out thanks to the snippet of the ONLY thing that's broken which you provided, knowing how Joomla works and git blame. There's also no confusion on my part about whether you are over–exaggerating what is broken and why you are doing it. No, Brian, not all aria-labelledby attributes are broken, it's just the search tools. The forms themselves are correctly generated and you have already nitpicked this to oblivion. I think the reason why you are making a mountain out of a molehill is pretty obvious to anyone who kept track of the discussion prior to my PR. Now be a grown ass man and fix the bug you introduced. Hell, I even told you how!

I am unsubscribing from this issue's notifications. I have better things to do on a Sunday evening than squabble over petty office politics.

avatar brianteeman
brianteeman - comment - 5 Dec 2021

No idea why the personal attacks. I have better things to do than fix bugs you create. We all know by now that your code is always perfect. I should have remembered that. I forgot.

avatar brianteeman
brianteeman - comment - 5 Dec 2021

Closed

avatar bembelimen bembelimen - close - 6 Dec 2021
avatar bembelimen
bembelimen - comment - 6 Dec 2021

Thank you both for finding the issue and proposing the fix. I've created a PR here: #36225 + some CSS adjustments.

avatar bembelimen bembelimen - change - 6 Dec 2021
Status New Closed
Closed_Date 0000-00-00 00:00:00 2021-12-06 01:03:13
Closed_By bembelimen
avatar richard67 richard67 - change - 6 Dec 2021
Labels Removed: Release Blocker
avatar richard67 richard67 - unlabeled - 6 Dec 2021

Add a Comment

Login with GitHub to post a comment