Page MenuHomePhabricator

Investigate: Grey background for temporary usernames in signatures, mentions etc
Open, Needs TriagePublic

Description

Motivation

Temporary usernames will feature a grey background when they appear in history pages, log pages etc (T325768: Design the visual look for temp usernames). For consistency, it is ideal that temporary usernames feature the same grey background when they appear in other places such as:

  • Signatures
  • Mentions

Is this possible? What is the work involved?

Event Timeline

Signatures and mentions are both just plain wikitext links:

  • Signatures: [[Special:Contributions:~2024-403|~2024-403]]
  • Mentions: [[User:2024-403|2024-403]]

Some options in no particular order:

1. Adding <span>s to the wikitext

Theoretically signatures could be updated to include a surrounding CSS class in the substituted wikitext:

<span class="mw-tempuser">[[Special:Contributions:~2024-403|~2024-403]]</span>

but this is getting quite cluttered for wikitext readers.

Mentions could be updated at least in DiscussionTools to generate more syntax, but mentions are also manually created by users or with templates, so that wouldn't catch all cases.

2. Modifying the parser

The parsers (both Parsoid and legacy PHP parser would need to be updated) can add CSS classes based on the target of a link, for example links to pages that don't exist get the new class.

Someone from the content transform team would be better placed to comment on the complexity and feasibility of this, but it would probably be the neatest solution, as the mw-tempuser link could be added at parse-time, if the target of the link is a user/user-talk/user-contributions page where the user is temporary.

(cc @cscott, @ssastry)

3. CSS hacks

Given the current output of this wikitext:

  • Signatures:
<a href="/wiki/Special:Contributions/~2024-403" title="Special:Contributions/~2024-403">~2024-403</a>
  • Mentions:
@<a href="/wiki/Benutzer:~2024-403" title="Benutzer:~2024-403">~2024-403</a>

we could look for the fairly rare strings /~2 or :~2 in the link href:

a[ href *= "/~2" ], a[ href *= ":~2" ] {
    background-color: #eaecf0;
    padding: 2px 8px 2px 8px;
    border-radius: 2px;
}

Cons:

  • Very hacky, and maybe a bit flaky
  • Could be false positives as it will target any link that contains /<tempusername>, e.g. [[Arbcom investigation/~2024-403]]

Pros:

  • By far the easiest of the three, could be deployed now with the code above.

The GetLinkColours hook was pretty much made for this -- adding extra classes to links, based on some function of the link title (prefixed DB key). There are four places in the code where onGetLinkColours is called, you could either change all four of those places to add the extra case for temporary account, or else just directly register a 'GetLinkColours' hook. If you went the later route, you could probably avoid some code duplication by handling redirects in the same generic hook.

$ git grep '>onGetLinkColours'
includes/api/ApiQueryInfo.php:          $this->getHookRunner()->onGetLinkColours(
includes/parser/LinkHolderArray.php:                    $this->hookRunner->onGetLinkColours( $pagemap, $classes, $this->parent->getTitle() );
includes/parser/LinkHolderArray.php:            $this->hookRunner->onGetLinkColours( $pagemap, $classes, $this->parent->getTitle() );
includes/parser/Parsoid/Config/DataAccess.php:          $this->hookRunner->onGetLinkColours(

You could also refactor to add a new LinkColor.php class with a LinkColor::getLinkColors() method which handles (a) redirects, (b) temporary accounts, and then (c) calls the onLinkColour() hook, all in one place. Then replace all the places which currently call the GetLinkColours hook with calls to the new centralized LinkColor::getLinkColors() method.

There's also LinkRenderer::getLinkClasses() but that doesn't seem to be consistently called by all the places (it's not used by Parsoid, eg) -- it seems to *just* do the redirect "color", which is done in a different way by Parsoid and ApiQueryInfo. Whatever place you put the new code, you'd want to be sure that all four places that currently call the GetLinkColours hook (which includes Parsoid in its DataAccess class) also add your new 'temporary account' class.

The catch is that the existing GetLinkColors hook takes as an argument a map from page ids to page title (prefixed DB key), and Special pages (like the temporary accounts) don't have a page ID and so can't get passed to this hook. Gah.

So I think just adding special handling to the four different places which handle link colors is probably the best route: LinkHolderArray already has a special case for ns == NS_SPECIAL, so adding another if statement which tests for temporary accounts should be straightforward, then the other two places (ApiQueryInfo and DataAccess) both do special case handling for redirects already so you'd just special case handling for temporary accounts in the same place. Maybe refactor those into a common function for added points.

An alternative we discussed is adding a new Hook, "GetLinkColorsToo" (or whatever) that just inverted the map, mapping from title to pageid instead of the reverse. Then the Disambiguator extension (the only user of the GetLinkColours hook in production) could just call array_values() instead of array_keys() to get the page ids for its db query, and we could pass in *all* the links (not just "links which have pageids") to the hook. But not sure it's worth doing all of that.

From slack:

@Esanders : It looks like in the legacy parser, special page links aren't up for replacement at all

<p><!--LINK'" 0:0-->
</p><p><!--LINK'" 0:1-->
</p><p><a href="/MediaWiki/core/index.php/Special:Contributions/~2024-123" title="Special:Contributions/~2024-123">Special:Contributions/~2024-123</a>
</p><p><!--LINK'" 2:2-->
</p>

^This is the pre-regex-replace HTML
My other links (to an existing page, a non-existing page and a user page) show up as replaceable comments
Interestingly the code for Special pages adds the "new" class, which would be incorrect, but the code never runs, so not sure what it is for...

@cscott: huh, weird. Parsoid just bypassed the whole "LinkHolder" abstraction. We'll probably have to grapple with it someday, but it's a big old ball of complexity.
So: straightforward to add classes to the ApiQueryInfo class and to Parsoid -- I verified that Parsoid passes special pages through to DataAccess. Might be a little trickier to do it for the legacy parser because special page links don't go through the "normal" link color mechanism.

@Esanders : Also LinkRenderer::makeBrokenLink hard codes [ 'class' => 'new' ], so there is no way to add a CSS class to a red link....
actually.. maybe with extraAttribs...
(very hacky though)

Change 1003080 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/core@master] [WIP] Add mw-tempuserlink to signatures and mentions in Parser

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

In T325768, the appearance was specified for temp user links in history pages, which adds 2px/8px padding, and this was implemented in the .mw-tempuserlink class as the default.

While this padding works on history pages, I think we need less in content pages, as it may be adjacent to other text, e.g.

in mentions:

image.png (38×305 px, 4 KB)

and to a lesser extent in signatures:

image.png (68×444 px, 13 KB)