Unnecessary aria-describedby attribute causes WCAG scans to fail

[ssalyers]

Subject of the issue

Valid form elements get aria-describedby added to them when they shouldn't

Your environment

• 1.17.0
• Chrome 61.0.3163.100 & Firefox 56.0.1

Steps to reproduce

1 - Create a form with at least 2 fields with validation on them
2 - Inspect the form fields and verify that they do not have aria-describedby attributes
3 - Submit the form with at least one field value that passes validation and at least one that does not, so the form doesn't actually post
4 - Inspect the valid form field

Expected behaviour

Nothing pointing to an error ought to change or be added to the field, since it's valid

Actual behaviour

The valid form field has had the aria-describedby attribute added, though the fieldname-error HTML element it references doesn't exist, since no error message was necessary. A WCAG/ADA scanner run at that point (for example, the aXe Chrome plugin) will flag the valid field as having a problem because the aria-describedby value is invalid

I corrected this locally by adding a check if message is null in the showLabel method before adding that attribute. That doesn't seem to have hurt anything, but counting on the absence of a message doesn't seem great. Some way to check if the element is valid before adding the new attribute would be great.

[staabm]

I thought we fixed this with a recent release...

[Arkni]

Just looked into the issue and tried the provided steps, I see no aria-describedby attribute added to valid elements.

Can you please provide a reproducible example so we can look into it.

Thanks!

[staabm]

Will close for now because of inactivity. Feel free to ask further questions/provide furhter information regarding this topic here. We can re-open if required.

[chrisgreen500]

Hi,

I too have been experiencing this problem.

I have attached a couple of images each showing shows a couple of divs with label and associated input tags: before shows the values prior to populating the CompanyName field, and after (unsurprisingly) shows the values after I've tabbed out (note: there is a value in CompanyName so it is not in error)

Before

After

I hope this helps.

jQuery validation: 1.17.0
Chrome: 62.0.3202.94

[luisluix]

I would like to get attention for this issue, caused by this line of code (please reopen):

$(element).attr("aria-describedby", describedBy);

That line makes it so aria-describedby is always added. When jquery tries to validate, after validation aria-describedby will be added regardless of the status of the errors, thus pointing to nothing when there are no errors.

I tried to fix it myself, but im missing code, that may require some regex.

//check if error messages arent empty
if (typeof message !== "undefined") {
  $(element).attr("aria-describedby", describedBy);
}
else {
    //remove reference to error div, but be careful since aria-describedby
    //could have been pointing to something other elements as well.

    /*
	TODO: check for the describedBy element, and remove it from the
			aria-describedby list without removing other elements in the list
    */

    //if aria-describedby is empty, remove it to avoid WCAG issues
    if (!$(element).attr("aria-describedby").length) {
	$(element).removeAttr("aria-describedby");
    }
}

Hope this helps

jQuery validation: 1.17.0

[Arkni]

Hi @chrisgreen500 and @luisluix

We will try to reinvestigate the issue another time later this week. But if any of you would like to provide a working example containing a set of reproducible steps on JSFiddle or JSBin, that will really help us a lot as we don't have the bandwidth for investigating all the issues right now.

Even better, if someone would like to write a patch for this, I'd be more than happy to walk you through the steps involved.

Thanks.

[luisluix]

Here you go, although not sure if this might be caused by unobtrusive, or by .net itself:

https://dotnetfiddle.net/HiDCu5

Step 1; click submit
Step 2: put something in textbox 1
Step 3: inspect elements

If you check the "aria-described by" of the elemements, input1 aria-described by points to "inputtext-error", but since we already fixed the error, the element with the "inputtext-error" ID no longer exists. This is what is causing the WCAG error. When we do step 2, aria-described by on input1 should be removed as well.

I posted this issue for unobtrusive as well, feel free to close it if it doesnt belongs here.

[Arkni]

@luisluix

Thanks for the example. I will check it later this week.

[abbottmw]

Here is another example that I am experiencing.

Net Fiddle

Steps to reproduce:

1. Right click on "Test Me" label and view in dev tools. There isnt an aria-describedby attribute on the input field below the label.
2. With the dev tools now open and in the area of the form field, go back and right click on the form field in the fiddle and choose inspect. You will see it adds an aria-describedby to a TestMe-error node that doesnt exist. This causes the WCAG scan to fail.

Before:

<input class="form-control" data-val="true" data-val-required="Error test" id="TestMe" name="TestMe" type="text" value="Hola">
<span class="field-validation-valid" data-valmsg-for="TestMe" data-valmsg-replace="true"></span>

After:

<input class="form-control valid" data-val="true" data-val-required="Error test" id="TestMe" name="TestMe" type="text" value="Hola" aria-describedby="TestMe-error" aria-invalid="false">
<span class="field-validation-valid" data-valmsg-for="TestMe" data-valmsg-replace="true"></span>

Hope this helps. it doesn't seem to add it when you are just focused in the input field, but when I go in and inspect it, it looks like it is firing some part of the validation.

[Arkni]

Hi @luisluix and @abbottmw,

Thanks a lot for the examples. I will work on a fix as soon as possible. Otherwise, as I said in a previous comment, if someone would like to write a patch for this, I'd be more than happy to walk you through the steps involved.

[abbottmw]

Would the steps involved be more or less wrapping an if statement around the block where it adds the aria-describedby to the element and removing it in an else? I do realize there would be a little more to it, but generally speaking is that all that needs to be done?

I am willing to take a stab at it, but would need to know all the caveats I would need to be aware of.

[abbottmw]

What about something similar to this? I was looking at the cleanup functions, but not sure if something easier can be used. Not sure about how this works if the element is grouped. Havent tested that part with this.

 if (typeof message !== "undefined") {
      $(element).attr("aria-describedby", describedBy);
  } else {
       describedBy = describedBy.replace(new RegExp("\\b" + this.escapeCssMeta(errorID) + "\\b"), "");
       //clean up attributes.  remove extra whitespace if exists
       describedBy = describedBy.replace(/\s+(?=\s)/g, '');
       if (describedBy.length) {
              $(element).attr("aria-describedby", describedBy);
       } else {
             $(element).removeAttr("aria-describedby");
        }     
   }

Update:
In my further testing, if the element already has an aria-describedby and there is a space at the end ( aria-describedby="foo ", then the validator will fail. Ideally, this will never happen, but I was trying to break it.

I went in and added some whitespace trimming of the start and end. Can I just add a trim() to the escapeCssMeta function to fix it across the board ?

[Arkni]

Hi @abbottmw

Sorry for the delayed answer. And thank you for your interest in fixing this issue.

You are on the right path.

Basically, you should split the value of the aria-describedby attribute using String#split() method. Then, get the index of the ID of the error element and remove it using the method Array#splice().

Finally, you join the remaining ids (if there still some) and reassign the new value to aria-describedby attribue. Otherwise, we should remove it if it was the error element was the only referenced element.

Sample code to help you with the task (you will not need to do the trimming):

// We split the aria-describedby value to multiple Ids
var describedByIds = describedBy.split( " " );

// Get the index of our error element
var index = $.inArray( describedByIds, errorID );

// Remove it if it exist
if ( index > -1 ) {
  describedByIds.splice( index, 1 );
}

// Replace the value of aria-describedby
if ( describedByIds.length ) {
  $( element ).attr( "aria-describedby", describedByIds.join( " " ) );

// Or remove the attribute if our error is the only referenced element
} else {
  $( element ).removeAttr( "aria-describedby" );
}

Also, we will need to change the method errorsFor() to use the element name concatenated to -error as part of the selector in case the aria-describedby attribute is not assigned a value.

errorsFor: function( element ) {
  var name = this.escapeCssMeta( this.idOrName( element ) ),
    describer = $( element ).attr( "aria-describedby" ),
    selector = "label[for='" + name + "'], label[for='" + name + "'] *";

  // 'aria-describedby' should directly reference the error element
  if ( describer ) {
    selector = selector + ", #" + this.escapeCssMeta( describer )
      .replace( /\s+/g, ", #" );
+ } else {
+   selector = selector + ", #" + name + "-error";
  }

  return this
    .errors()
    .filter( selector );
},

You will also need to adjust the related tests in test/error-placement.js file.

I went in and added some whitespace trimming of the start and end. Can I just add a trim() to the escapeCssMeta function to fix it across the board ?

Adding trimming ability to escapeCssMeta defeat its purpose. That method should remain doing only the escaping and nothing more.

[cameron-gray]

Are there any updates on this? According to the release notes for 1.18.0, the fix for this bug did not make the build. I glanced at the commits to master since the release of 1.18.0, and it doesn't appear that any commits are related to this bug.

As ADA compliance becomes more of a requirement rather than a suggestion, I worry a bit that this bug has been outstanding for quite some time. That said, I completely understand that no one is getting paid to work on and maintain this project...just looking for any updates. :)

We are using v1.17.0 right now.

[Arkni]

Hi @cameron-gray,

Thanks a lot for the reminder! time flies, It's been almost a year since my last comment. Lately I have been very busy, so I wont promise you anything, but I will try my best to implement the changes I mentioned in my comment #2094 (comment) as soon as I got some free time, unless someone beat me to it.

Thanks!

[Mogget24]

Hi
I had the same problem today and I've found out that the error was caused by the 'errorPlacement' setting passed to the validator defaults

errorPlacement: (err, elm) => {
const $error = $(elm).find('.error-message');

   [...]

    $error.text(...).attr('id', err.attr('id'));
  },

The 'id' attribute is required for having this to work; in fact this ID is given to the element pointed by the aria-describedby attribute given to the element with the error

Hope this might help
Bests, Michael

[teambalthazar]

Are there any updates on this? According to the release notes for 1.18.0, the fix for this bug did not make the build. I glanced at the commits to master since the release of 1.18.0, and it doesn't appear that any commits are related to this bug.

As ADA compliance becomes more of a requirement rather than a suggestion, I worry a bit that this bug has been outstanding for quite some time. That said, I completely understand that no one is getting paid to work on and maintain this project...just looking for any updates. :)

We are using v1.17.0 right now.

Just wanted to add that this has been an issue flagged by our ADA compliance partner. This issue has been and will be marked on our WCAG statement.

[pkunze]

We managed to work around this issue by using:

var advancedValidationSettings = {
	success: function (label, element) {
		var describedBy = $(element).attr("aria-describedBy")
			.replace(new RegExp("(.*)\\s+" + element.id + "-error(.*)"), "$1 $2")
			.trim();

		if (describedBy) {
			$(element).attr("aria-describedBy", describedBy);
		} else {
			$(element).removeAttr("aria-describedBy");
		}
	}
};

$.validator.unobtrusive.options = advancedValidationSettings;

Maybe this helps somebody in the meantime.

[vince844]

Got same problem in cloned fields, solved by removing the attribute just after cloning fields. Gave me a bit of headache though

[thomastwosome]

We managed to work around this issue by using:
var advancedValidationSettings = {
success: function (label, element) {
var describedBy = $(element).attr("aria-describedBy")
.replace(new RegExp("(.)\s+" + element.id + "-error(.)"), "$1 $2")
.trim();

  if (describedBy) {
  	$(element).attr("aria-describedBy", describedBy);
  } else {
  	$(element).removeAttr("aria-describedBy");
  }

}
};

$.validator.unobtrusive.options = advancedValidationSettings;
Maybe this helps somebody in the meantime.

I'm having this same exact issue. Where would I place this code? I'm a newbie. Thank you!

[pkunze]

@thomastwosome basically anywhere after you included jquery.validation. For example in a new script-tag at the bottom of your site.

[ajithvs]

Hi @Arkni
Any update on this?
We are using jQuery.Validation -Version 1.19.1
I am getting a similar issue when I scan the pages.

Also, another issue is the validation is not triggering like the old version 1.13.0 when a control value changes.
Scenario
Enter something in the text box and clear the same field value without moving the mouse cursor by pressing the backspace key.. This time in the old version will show a validation message. but in new version which is not triggering a validation message.

Any solution for this?

@pkunze - I will try this. Thanks a lot.

[Arkni]

Hi Ajith, Unfortunately no. I wasn't able to be active the last two years due to some health issues and probably will not be able to allocate enough time for Open Source for the next few months. I hope someone will try to fix this in the near future and can get inspiration from one of my previous comments I shared two years ago. As for the second issue, the validation was changed from eager to lazy after 1.13.0, but I don't remember as of which version exactly. You may find an answer in the changelog. Hope this answers your question. Thanks,

…

On Fri, 26 Jun 2020 at 17:26, Ajith ***@***.***> wrote: Hi @Arkni <https://github.com/Arkni> Any update on this? We are using jQuery.Validation -Version 1.19.1 I am getting a similar issue when I scan the pages. Also, another issue is the validation is not triggering like the old version 1.13.0 when a control value changes. Scenario Enter something in the text box and clear the same field value without moving the mouse cursor by pressing the backspace key.. This time in the old version will show a validation message. but in new version which is not triggering a validation message. Any solution for this? @pkunze <https://github.com/pkunze> - I will try this. Thanks a lot. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2094 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABURU6345TNZX7IL77JYCFDRYTD3NANCNFSM4D7Z4MXQ> .

[eden-jh]

I have an update almost ready that adds an ariaDescribedbyCleanup option to the validator settings (with a default of false). If set to true, it will remove the error ID from an element's aria-describedby when that error is being hidden. I figured it was safest to have it default to the current behavior, but I don't feel that strongly about it (or about the name of the option).

Updating unobtrusive to include this setting should resolve the issue there, though I won't be able to make the change myself.

I'm adding one or two more unit tests, after which I should be ready to submit a PR.

[elezar42]

@bytestream It seems that you have taken over as the maintainer of this project. Do you think it would be possible to review the PR that eden-jh created (#2410)? It was created in the time between when Markus & Brahim were maintaining the repo and when you started to do so, and it was auto-closed for being stale. I can submit a new PR for it, but if the existing one would still be applicable, it would be easier to just re-open it and merge it.