Centralize temporary IP masking user logic inside MediaWiki core
Closed, ResolvedPublicBUG REPORT
Actions

Assigned To

Authored By

	Jdlrobson
	Feb 3 2023, 1:16 AM

Description

We have some code in Vector skin for handling IP masked users
This isn't well maintained and has already broken (user page link does not show on desktop view and is styled incorrectly on tablet view as per screen) in the short term it's been there

Screen Shot 2023-02-02 at 4.49.37 PM.png (508×900 px, 63 KB)

I'd like to move the logic for the temporary user account into the shared Skin class. This will avoid us having to update every skin to handle the new UI and will allow skins to not worry about the IP masking change unless they want to.

After proposed change without any modifications to skins

Skin	Full screen	Lower resolution
Vector 2022.
Vector		N/A
Timeless
Modern.		N/A
CologneBlue		N/A
Pivot
Monobook	.
Minerva.		[1]

[1] (Updating the icon in Minerva should be relatively trivial)

Details

Subject	Repo	Branch	Lines +/-
Skins: Login link should be shown to temporary users by default	mediawiki/core	master	+1 -1
2010: IP masked user icon should differ	mediawiki/skins/Vector	master	+10 -0
Revert "Updates for core temp user autocreation feature"	mediawiki/skins/Vector	master	+27 -40
Centralize temporary IP masking user logic inside MediaWiki core	mediawiki/core	master	+18 -17

Customize query in gerrit

Related Objects

Mentioned Here: T326911: Update Web Team-owned products that may be affected by IP Masking

Event Timeline

Jdlrobson created this task.Feb 3 2023, 1:16 AM

Restricted Application added subscribers: Masumrezarock100, Aklapper. · View Herald TranscriptFeb 3 2023, 1:16 AM

Change 886189 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Centralize temporary IP masking user logic inside MediaWiki core

https://gerrit.wikimedia.org/r/886189

Change 886190 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Revert "Updates for core temp user autocreation feature"

https://gerrit.wikimedia.org/r/886190

Hey @tstarling I think this would minimize the migration work we need to do on the skins-side while preserving the UX changes you've already made for Vector. Let me know if anything doesn't make sense here and you want to chat further.

alistair3149 subscribed.Feb 3 2023, 3:26 AM

Jdlrobson moved this task from Backlog to Skin components on the MediaWiki-Core-Skin-Architecture board.Feb 9 2023, 12:21 AM

It doesn't really preserve the UX changes I did. I asked @Prtksxna if the precise styling of user links in accordance with the Figma mockup was an essential requirement, because it's hard to do that. In particular, I asked if they have to be grey and unlinked, because it's obviously much easier to make them be blue links with href="#". Prateek responded that it is indeed necessary, and so I wrote the skin patches to conform to that. Your patch removes the work I did on that requirement.

Change 886189 merged by jenkins-bot:

[mediawiki/core@master] Centralize temporary IP masking user logic inside MediaWiki core

https://gerrit.wikimedia.org/r/886189

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.25; 2023-02-27).Feb 21 2023, 4:00 AM

Change 886190 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Revert "Updates for core temp user autocreation feature"

https://gerrit.wikimedia.org/r/886190

Maintenance_bot removed a project: Patch-For-Review.Feb 22 2023, 11:10 PM

In T328725#8631270, @tstarling wrote:

It doesn't really preserve the UX changes I did. I asked @Prtksxna if the precise styling of user links in accordance with the Figma mockup was an essential requirement, because it's hard to do that. In particular, I asked if they have to be grey and unlinked, because it's obviously much easier to make them be blue links with href="#". Prateek responded that it is indeed necessary, and so I wrote the skin patches to conform to that. Your patch removes the work I did on that requirement.

No UX changes were intended as part of this change. As I pointed out in the description the UX was already broken. I'll look into this tomorrow making sure to align more with https://www.figma.com/file/xECM2hD7vEemP91M9YAQBC/IP-Masking?node-id=0%3A1

Jdlrobson claimed this task.Feb 23 2023, 5:26 PM

There needs to be a login link, for example in the vector-2022 user menu. It's not enough to just show "create account".

Change 891858 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Login link should be shown to temporary users by default

https://gerrit.wikimedia.org/r/891858

gerritbot added a project: Patch-For-Review.Feb 24 2023, 4:55 PM

Change 891859 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] 2010: IP masked user icon should differ

https://gerrit.wikimedia.org/r/891859

Thanks for flagging that @tstarling - I've updated the logic in core: https://gerrit.wikimedia.org/r/891858

I also have a patch to restore the styling changes to match the mock appearance in legacy Vector:: https://gerrit.wikimedia.org/r/891859

There are also some product decisions we'll need to make and document which are implied by the mock (apologies if these are answered elsewhere - feel free to merge as duplicate). See T326911#8644788 for more information.

Change 891858 merged by jenkins-bot:

[mediawiki/core@master] Skins: Login link should be shown to temporary users by default

https://gerrit.wikimedia.org/r/891858

Change 891859 merged by jenkins-bot: