Skip to content

Set appropriate autocomplete attributes on password fields#13078

Merged
lafriks merged 3 commits into
go-gitea:masterfrom
silverwind:new-pw
Oct 9, 2020
Merged

Set appropriate autocomplete attributes on password fields#13078
lafriks merged 3 commits into
go-gitea:masterfrom
silverwind:new-pw

Conversation

@silverwind
Copy link
Copy Markdown
Member

new-password prevents annoying autocompletion in some cases, thought it's not semantically correct to use that for example on all three fields on the user account page, so some annoyances remain.

`new-password` prevents annoying autocompletion in some cases, thought
it's not semantically correct to use that for example on all three
fields on the user account page, so some annoyances remain.
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Oct 8, 2020
@zeripath zeripath added the topic/ui Change the appearance of the Gitea UI label Oct 8, 2020
@zeripath zeripath added this to the 1.14.0 milestone Oct 8, 2020
@zeripath
Copy link
Copy Markdown
Contributor

zeripath commented Oct 8, 2020

I've set this as 1.14 but if it's approved before 1.13 is ready I think it would be reasonable to merge before.

@lafriks lafriks modified the milestones: 1.14.0, 1.13.0 Oct 9, 2020
Copy link
Copy Markdown
Member

@lafriks lafriks left a comment

Choose a reason for hiding this comment

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

All current browsers will still ignore autocomplete="off" but it will not hurt :)

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 9, 2020
@lafriks
Copy link
Copy Markdown
Member

lafriks commented Oct 9, 2020

🚀

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #13078 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13078      +/-   ##
==========================================
- Coverage   42.62%   42.61%   -0.02%     
==========================================
  Files         673      673              
  Lines       73858    73858              
==========================================
- Hits        31482    31474       -8     
- Misses      37268    37274       +6     
- Partials     5108     5110       +2     
Impacted Files Coverage Δ
modules/process/manager.go 72.50% <0.00%> (-2.50%) ⬇️
services/pull/check.go 47.69% <0.00%> (-2.31%) ⬇️
modules/log/event.go 59.90% <0.00%> (-1.89%) ⬇️
models/issue_comment.go 53.75% <0.00%> (-0.16%) ⬇️
models/error.go 34.85% <0.00%> (ø)
services/pull/pull.go 40.78% <0.00%> (ø)
modules/indexer/stats/db.go 60.86% <0.00%> (+8.69%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea69ec6...680be50. Read the comment docs.

@lafriks lafriks merged commit 1c523e2 into go-gitea:master Oct 9, 2020
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Oct 9, 2020

All current browsers will still ignore autocomplete="off"

I'm still hoping that some password managers will eventually start to honour it 😉

There's one minor remaining issue on the user account page that my password manager autofills current password so the form will be considered dirty by jquery-are-you sure. Maybe that field can be ignored from dirty check, will have to check later.

@silverwind silverwind deleted the new-pw branch October 9, 2020 07:41
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants