Show visibility status of email in own profile

[n0toose]

I've heard many reports of users getting scared when they see their own
email address for their own profile, as they believe that the email
field is also visible to other users. Currently, using Incognito mode
or going over the Settings is the only "reasonable" way to verify this
from the perspective of the user.

A locked padlock should be enough to indicate that the email is not
visible to anyone apart from the user and the admins. An unlocked
padlock is used if the email address is only shown to authenticated
users.

Some additional string-related changes in the Settings were introduced
as well to ensure consistency, and the comments in the relevant tests
were improved so as to allow for easier modifications in the future.

---

Screenshot (EDIT: Scroll down for more up-to-date screenshots)

Please remove this section before merging.

This lock should only appear if the email address is explicitly hidden using the Hide Email Address setting. The change was originally tested on top of and designed for the Forgejo fork, but I don't expect any problems to arise from this and I don't think that a documentation-related change is strictly necessary.

[n0toose]

I just made a force-push as I made a typo in the commit summary, apologies :D

[n0toose]

This change was apparently made redundant last week and I didn't precisely notice that change, apologies once again: 6706ac2

I'll revamp the PR as I believe that I could improve the UX a little bit with my own idea.

[n0toose]

Here's a new concept:

[n0toose]

Hi, sorry for the mess. I introduced the "show main email on the user's profile" thing again, but ended up using icons to clearly differentiate between public and no visibility. I think this is consistent with the overall look of the personalized Profile page, as similar indicators are present next to the users' repositories, and this leaves some room for additional, purely hypothetical, changes in the future, like e.g. "only users that are logged in can see my email".

I am not used to modifying templates, this is a first contribution and I see that the coverage has dropped. Feedback is appreciated.

The change was tested against the latest Gitea commit.

[silverwind]

Hmm, instead of adding an additional icon which does not really fit there, why not indicate on mouseover via data-tooltip-content? At minimum, I would move icon to the right side, but it still looks a bit odd imho.

[n0toose]

why not indicate on mouseover via data-tooltip-content

I don't think that non-obvious xkcd-style "hover over it" tips will be helpful when you e.g. think that your email address has been leaked (I'm comparing this to the situation before the aforementioned commit was merged). I think that not including the address at all, which is the case right now, would be relatively better.

[n0toose]

oh, uh, this seems to be breaking the tests now :D

[silverwind]

IMHO, add right-side icon https://primer.github.io/octicons/info-16, with data-tooltip-content via translation "Only logged-in users can see your e-mail address".

Also check if there are other conditions besides the logged-in condition. There might be an option to always hide the address from everyone but yourselves.

[n0toose]

IMHO, add right-side icon https://primer.github.io/octicons/info-16, with data-tooltip-content via translation "Only logged-in users can see your e-mail address".

I don't want to get super annoying about this, but I am not sure that not using clear iconography to describe the privacy status is productive. I can see this making a lot of sense with the icons that I am using right now, but I don't believe that an ℹ️ box with a hint should not adequately fix the "super scary and confusing UI" problem that I brought up - I'd like for a hint to play a complementary role to confirm what the user is seeing, instead of showing them something in such a confusing way that prompted 6706ac2 (aka. just the email address, with the addition of an omnipresent info box) and then telling them "oh, you're fine, everything is the way you want it (?)" in a smaller hint box.

[n0toose]

Also check if there are other conditions besides the logged-in condition. There might be an option to always hide the address from everyone but yourselves.

Oh, sorry, just noticed that I kind of messed up that part after I tried to scramble everything together to adjust my change so that it'd work with this codebase.

I did it the way I wanted to do it, but I also used hints and also a hyperlink that leads the user directly to the right checkbox.

Logged in

Logged out

(Email address is not supposed to be hidden here.)

[n0toose]

I'll push one final fix for the email-address-not-showing-up thing, started making some very trivial mistakes while trying to apply fixes.

[n0toose]

Hi again, thanks for your patience. The PR isn't ready yet, as for some reason, .ShowUserEmail doesn't work as it should anymore.

Accessing the user profile in Incognito mode when .ShowUserEmail is removed always shows the email address without the icons (so, the first condition fails, else-block is accessed properly) but re-adding it again means that the email address will never be shown, regardless of the setting that the user has set. I am not sure why this would break, as this is how this basically worked before. I'd very much appreciate any assistance and any further feedback :D

Works as intended, I forgot that the recent change (or not recent, the fundamental misunderstanding is still the same) made it so that email addresses are never shown to the general public, without a user account, by default.

[fogti]

so, I'd guess in the file linked above lines 79 and/or 88 need to be changed (e.g. s/NotContains/Contains/ on these lines). The tests appear to be failing for unrelated reasons (security advisory scanning, but this PR so far hasn't modified dependencies or actual non-template code, so I'd assume that it's unrelated), which are perpendicular to this PR.

if this problem is fixed in main, rebasing should help.

[n0toose]

I think

gitea/tests/integration/setting_test.go

Lines 64 to 65 in 376396a

	// Should not contain even if the user visits their own profile page
	assert.NotContains(t, htmlDoc.doc.Find(".user.profile").Text(), "user2@example.com")

and

gitea/tests/integration/setting_test.go

Lines 74 to 90 in 376396a

	// user2 can not see self
	session = loginUser(t, "user2")
	req = NewRequest(t, "GET", "/user2")
	resp = session.MakeRequest(t, req, http.StatusOK)
	htmlDoc = NewHTMLParser(t, resp.Body)
	assert.NotContains(t, htmlDoc.doc.Find(".user.profile").Text(), "user2@example.com")
	setting.UI.ShowUserEmail = false
	// user1 can not see self
	session = loginUser(t, "user1")
	req = NewRequest(t, "GET", "/user1")
	resp = session.MakeRequest(t, req, http.StatusOK)
	htmlDoc = NewHTMLParser(t, resp.Body)
	assert.NotContains(t, htmlDoc.doc.Find(".user.profile").Text(), "user1@example.com")
	setting.UI.ShowUserEmail = showUserEmail

have to be removed from the test cases without introducing anything new, as

gitea/tests/integration/setting_test.go

Lines 53 to 58 in 376396a

	// user1 can see self
	session := loginUser(t, "user1")
	req := NewRequest(t, "GET", "/user1")
	resp := session.MakeRequest(t, req, http.StatusOK)
	htmlDoc := NewHTMLParser(t, resp.Body)
	assert.Contains(t, htmlDoc.doc.Find(".user.profile").Text(), "user1@example.com")

checks whether the email is still present in the user profile.

Those lines should be removed and no additional checks should be introduced, if I am not wrong. I will take another careful look later, just wanted to post a small status update.

[n0toose]

Nevermind, I just changed the last part (// user2 can not see self) and improved the comments a little bit.

[n0toose]

(p.s. before squashing and merging, please check that the attribution using my full name is correct and that github isn't adding some Co-authored-by tag using my username instead of my full name, as the merge commit used my username instead, i'd rebase it all myself, but that would delete the previous review and potentially cause more trouble, ty ❤️)

[n0toose]

Maybe this string should be altered as well:

gitea/options/locale/locale_en-US.ini

Line 615 in 8117462

keep_activity_private_popup = Makes the activity visible only for you and the admins

[silverwind]

Yeah but that one is not related to this PR, would do in another PR.

[n0toose]

Got it.

[silverwind]

Co-authored-by

(p.s. before squashing and merging, please check that the attribution using my full name is correct and that github isn't adding some Co-authored-by tag using my username instead of my full name, as the merge commit used my username instead, i'd rebase it all myself, but that would delete the previous review and potentially cause more trouble, ty ❤️)

Checked the merge message, there is only a Co-authored-by for me, you are the author.

[n0toose]

Checked the merge message, there is only a Co-authored-by for me, you are the author.

if it contains the username n0toose and not full name as it is visible in the git log -p, I'd appreciate a correction :D

[silverwind]

if it contains the username n0toose and not full name as it is visible in the git log -p, I'd appreciate a correction :D

It contains your original username from the initial commit. I can't alter this on the UI here, and also would prefer not to :)

[n0toose]

Should be fine then, I'm just conditioned to double-checking that my information does not show up double / "two authors, one email" as e.g. Gitea does not handle that as well as GitHub and that ends up ruining things like internal contribution statistics. Sorry and thanks, I am just a bit of a perfectionist even if it doesn't matter in the end of the day :)