ayg wrote:

( "User talk:" . $title->getText() ) is unlocalized and fragile. Use $title->getTalkPage(), and the Obj variants of the appropriate functions. Also, you don't know the link exists, so getKnownLink isn't appropriate. The change should look more like

• $titleLink .= ' (' . $skin->makeKnownLinkObj( SpecialPage::getTitleFor( 'Contributions', $title->getDBkey() ), wfMsg( 'contribslink' ) ) . ')';

+ $titleLink .= ' (' . $skin->makeLinkObj( $title->getTalkPage(), wfMsg( 'talkpagelinktext' ) ) . " | " . $skin->makeKnownLinkObj( SpecialPage::getTitleFor( 'Contributions', $title->getDBkey() ), wfMsg( 'contribslink' ) ) . ')';

Except that the separator should be localized as well (although in other places it's not).

Really, is there any reason for this to not just be a system message so sysops can add any extra info they like?

fearow00 wrote:

I guess a sysop-editable format would be possible - maybe a system message, so they can do something like "[[User:$1|$1]] ([[User talk:$1|Talk]] | [[Special:Contributions/$1|contribs]])", however thats a bigger change and doesnt take into account localisation as easy.

ayg wrote:

It takes localization into account more easily if anything, since it allows the parentheses and other delimiters to be localized. It is a bigger change, yes, but not by a big amount.

fearow00 wrote:

Patch v2

This is a new patch, using a more-localised format. The | still isnt localised, but it isnt in many places.

attachment blockLogLinkTalk_v2.patch ignored as obsolete

ayg wrote:

Personally, I won't commit a patch for this that just hard-codes the change rather than using a system message, unless it's pointed out that there's some issue with that (I can't think what).

fearow00 wrote:

The current system is like that, so I wrote a patch that already uses the current system. If you want me to rewrite that bit of code as a newer system, using a system message, then I will do so. I'll work on it now.

fearow00 wrote:

New version

This version makes the entire thing be generated from a system message, [[MediaWiki:blockloguserlinks]]. It's default is "[[{{ns:user}}:$1|$1]] ([[{{ns:user_talk}}:$1|talk]] | [[Special:Contributions/$1|contribs]])". $1 is the username.

Attached:

ayg wrote:

Sorry to lead you on a wild goose chase (always a risk in these things), but in the bright light of the day, it occurs to me that it might not be ideal to call the parser 5000 times when generating a page.

Having done a little more investigation, I see that watchlist and recentchanges use Linker::userLink and Linker::userToolLinks. Those should be used here, too, for consistency. However, these currently require a user ID to be submitted, which is ridiculous. I'm looking at that to see if it can't be fixed.

ayg wrote:

(see also bug 10663)

fearow00 wrote:

Using userlink and toollinks is hardcoding - there is nowhere as much flexibility. Also, those do not work for anonymous editors, who just have the IP address and no ID.

Also, my version calls the parser one less time than otherwise - normally it has to link up the user links, the contribs link, etc. This just calls it once to generate the entire thing from a system message.

An option is to modify the userLink and userToolLinks to operate on a easier system, not requiring userids, and allowing customisation per a system message. I think that until a system such as bug 10663 is patched into the code, a system similair to what I have already is the easiest option.

robchur wrote:

If you had done your research, you would have discovered that anonymous users do, in fact, have identifiers - specifically, zero - and it is this fact which is significant to the referenced methods.

ayg wrote:

(In reply to comment #10)

Using userlink and toollinks is hardcoding - there is nowhere as much
flexibility.

There's more consistency. Flexibility can be added to those messages if desired, or to the skin itself.

Also, those do not work for anonymous editors, who just have the
IP address and no ID.

They do, as Rob points out.

Also, my version calls the parser one less time than otherwise - normally it
has to link up the user links, the contribs link, etc. This just calls it once
to generate the entire thing from a system message.

Linker::makeLink does not call the parser. You should understand this if you're working with MediaWiki: Parser is a massive, bloated atrocity that takes forever to do anything. At last count I think it took 800 ms to parse a typical full page. The parser takes up some 40% of CPU time from MW last I heard. Therefore, we avoid it if we can, and we definitely do not ever call it 5000 or even 50 times per page. That would simply cause PHP to time out and the page to return blank.

An option is to modify the userLink and userToolLinks to operate on a easier
system, not requiring userids, and allowing customisation per a system message.
I think that until a system such as bug 10663 is patched into the code, a
system similair to what I have already is the easiest option.

Not if we're going to change it yet again later. For the time being this could just pass 0 for anonymous users and 1 for logged-in users, which will cause the functions to work correctly despite their documentation.

fearow00 wrote:

I am aware that anonymous users have a userid of 0, I meant that they don't have their own userid. I was under the impression the userlinks needed the ID for getting data - I posted that before fully reading about it. Regarding the parser - I also assumed makeLink did a similair thing - that's my mistake there. If we need to have flexibility in the linking here, using a system message, is the parser necessary? Or is there another easy way of customising what the links are without the parser?

ayg wrote:

For now let's just stick with the current issue, and worry about customizability for *all* userToolLinks uses later.

robchur wrote:

Fixed in r24612.