Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in showLabel method - creates duplicate aria-describedby elements #1200

Closed
rodgerramjet opened this issue Jul 6, 2014 · 8 comments
Closed

Comments

@rodgerramjet
Copy link

Found in 1.13.0

This code (I assume) is trying to send the \b to the regex, but its being escaped by the JS

Should be something like "\b" + errorID + "\b"?

CODE:
} else if ( !describedBy.match( new RegExp( "\b" + errorID + "\b" ) ) ) {
// Add to end of list if not already present
describedBy += " " + errorID;
}

This creates loads of duplicate aria-describedby in the markup

@staabm
Copy link
Member

staabm commented Jul 6, 2014

//cc @tractorcow

@tractorcow
Copy link
Collaborator

My apologies for this error. While this is a bug, it didn't appear in testing due to the incorrect code being bypassed if showLabel could detect an existing error label to place this element into. When I wrote a few test cases I found this issue only could be replicated if I placed the aria-describedby reference prior to validation being setup. I hope the test case correctly captures your scenario.

@staabm
Copy link
Member

staabm commented Jul 6, 2014

@davidhedges could you verify that @tractorcow's PR fixes your issue?

@rodgerramjet
Copy link
Author

Yep that fixes it- thanks guys :)

In terms of how I reproduced it- I haven't gone into too much detail - we are using jquery validate, unobtrusive validation, so a lot of stuff is getting done behind the scenes.

Here is a JSFiddle I threw together for you. Not sure if its an unobtrusive bug exacerbating your bug or not?

http://jsfiddle.net/cW9n5/

@rodgerramjet
Copy link
Author

perhaps your test case should be updated to this scenario?

@rodgerramjet
Copy link
Author

Updated Jsfiddle to show aria-described by on the page- test it by flling in invalid values & tabbing in and out of the field
http://jsfiddle.net/cW9n5/1/

@tractorcow
Copy link
Collaborator

It looks like this example breaks the code because there is no error label, so showError continually attempts to re-place it on the page. Is this the behaviour that unobtrusive validation circumvents?

@tractorcow
Copy link
Collaborator

I'm not sure we could update this test case to depend on third party libraries, if it is the case that this is necessary to replicate the full error. jquery.validate.unobtrusive.js isn't a supported module is it? The new test case I added failed under the old code, so that's probably enough (as far as our responsibility goes here) to validate we've fixed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants