Skip to content
New issue

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

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

Already on GitHub? # to your account

Use the autocomplete attribute as a hint #723

Merged
merged 1 commit into from
Feb 10, 2020

Conversation

stefansundin
Copy link
Contributor

The change: Do not add helpers on fields that have autocomplete="off" or autocomplete="new-password". Identify fields with autocomplete="one-time-code" as TOTP fields.

Feel free to change things such as function naming, etc. I hope this PR is a good demonstration of what I am looking for. I used this MDN page: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete

I think it would also be good to somehow be able to configure how aggressive the extension should be in trying to identify things. Sometimes I see the helpers on way too many fields and it would be nice to maybe have a "strict" mode option? Remember how browsers used to have (or still have?) a Quirks mode and Strict mode.

Anyhow, I hope you consider this. Thanks!

@varjolintu
Copy link
Member

It needs to be tested that saving new credentials still works if new-password ignores the helpers.

Also, please use === as a strict comparison instead of ==.

One possible change for a "strict" mode would be that it doesn't add any new helpers after a login input field combination has been detected, unless it's happening via MutationObserver. Still, this could be just a modification to the default behaviour.

Copy link
Member

@varjolintu varjolintu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do the necessary changes requested in the previous message.

@stefansundin
Copy link
Contributor Author

Hi @varjolintu. I made the change to ===. I am not sure the best way to test if saving new credentials will still work for autocomplete="new-password". Can you test it?

And you are able to make changes in my branch, in case more changes are necessary. Please feel free to do so. Thanks.

@varjolintu
Copy link
Member

Just tested it. Works great!

@varjolintu varjolintu added this to the 1.5.5 milestone Feb 3, 2020
@varjolintu
Copy link
Member

Can you squash the commits? Thanks.

…lete="new-password". Identify fields with autocomplete="one-time-code" as TOTP fields.
@stefansundin
Copy link
Contributor Author

@varjolintu Done. GitHub also has a squash and merge feature (if you press the arrow next to the merge button). It also let's you change the commit message.

@varjolintu
Copy link
Member

Thanks! I always forget that feature exists..

@varjolintu varjolintu merged commit e1e5da2 into keepassxreboot:develop Feb 10, 2020
@varjolintu varjolintu mentioned this pull request Feb 20, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants