-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
add audit to check if paste is allowed in password inputs #2366
add audit to check if paste is allowed in password inputs #2366
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
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.
this is awesome stuff thanks @robin-drexler! 🎉
when you start on adding output, we can follow the accessibility audits model of showing the outerHTML of the element (can take a look at the axe-audit.js in audits/accessibility for the output object format, feel free to ignore the proprietary "path" part for now :) )
for tests, a smoke test in the dobetterweb tester would be perfect too 👌
feel free to reach out to us for any help if you get stuck and thanks for the contribution!
nice nice! this is on point so far. well done. :) |
I just didn't look thoroughly enough, sorry. There was a linting issue which should now be resolved. |
395644e
to
89c93d3
Compare
I signed it! |
CLAs look good, thanks! |
Should I also create a doc page like https://developers.google.com/web/tools/lighthouse/audits/geolocation-on-load ? If that's the case, can somebody point me to the respective repo? :) |
thanks @robin-drexler! You can find the documentation here https://github.com/google/WebFundamentals/tree/master/src/content/en/tools/lighthouse/audits cc @ebidel |
lighthouse-core/config/default.js
Outdated
@@ -133,7 +134,8 @@ module.exports = { | |||
"dobetterweb/notification-on-start", | |||
"dobetterweb/script-blocking-first-paint", | |||
"dobetterweb/uses-http2", | |||
"dobetterweb/uses-passive-event-listeners" | |||
"dobetterweb/uses-passive-event-listeners", | |||
"dobetterweb/password-inputs-can-be-pasted-into" |
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.
nit, all these audits are arranged alphabetical, maybe order them here too
lighthouse-core/config/default.js
Outdated
@@ -26,6 +26,7 @@ module.exports = { | |||
"dobetterweb/response-compression", | |||
"dobetterweb/tags-blocking-first-paint", | |||
"dobetterweb/websql", | |||
"dobetterweb/password-inputs-with-prevented-paste", |
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.
nit, all these audits are arranged alphabetical, maybe order them here too
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
I'd like to finish this during the weekend but probably could use some help with copywriting and docs and some hints on what else might need to be done. Could anyone chime in? :) |
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.
Robin, this is so good.
Everything here is in its right place and I got nothing but praise for this PR.
Huge. Thank you! 🥇 🥇 🥇 |
Fantastic, can't wait to see it in a release. :D |
all good! sorry for not being timely in our replies. |
Adresses #2345
todo