-
Notifications
You must be signed in to change notification settings - Fork 78
"Autocomplete attribute has valid value" [73f2c2]: Add more failing examples #2032
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
Conversation
The additional tests look good to me, but I would want to get confirmation that they create actual problems before approving. Otherwise, there may be additional details we need to add into the background or accessibility support. |
Ah, yes. Haven't really checked that (not sure how to… 😅 ) If they do not create a problem, then we should instead update the rule (+note saying why we accept them), and have them as Passed/Inapplicable examples, I guess (currently, they are failing the rule as it is written). If they are inconsistent dependant on UA/AT combination, we should replace them with an Accessibility Support note. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know that the test suite necessarily needs to be this extensive. I don't mind if they are though, so fine with me.
As for Trevor's suggestion, I'm not sure what there is to test about fail 6 - 9. They are missing a key token, so they couldn't possibly work. For 9 and 10 you could publish a page somewhere where you check if these can be autocompleted by the browser. We did test this at the time we wrote the rule, I have no reason to think this changed so for me that's not strictly necessary.
I recently figured out that Alfa was passing these examples, and getting a consistent implementation of the rule despite that problem 😅 |
Call for review ends on March 23rd. |
Call for review has ended. Merging. |
While working on Alfa support for
webauthn
, we found out that there was a bunch of incorrect scenario we were not detecting 🙈 So adding some more failed examples to catch these.Not that theHTML spec for the tokens states: (emphasise mine)
The "just" bit is my reasoning for failing case with extra tokens at the end. I haven't check if these cause actual problems with autofilling 😅 If they don't, we should rather update the Expectation to state that they are not problematic.
Closes issue(s):
Need for Call for Review:
This will require a 1 week Call for Review (adding a bunch of examples).
Pull Request Etiquette
When creating PR:
develop
branch (left side).After creating PR:
Rule
,Definition
orChore
.When merging a PR:
How to Review And Approve