Last modified: 2007-08-06 07:10:21 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T12655, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 10655 - Link to talk page in block log message
Link to talk page in block log message
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Interface (Other open bugs)
1.11.x
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch, patch-need-review
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-22 05:28 UTC by Matt
Modified: 2007-08-06 07:10 UTC (History)
0 users

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
The patch (785 bytes, patch)
2007-07-22 05:28 UTC, Matt
Details
Patch v2 (768 bytes, patch)
2007-07-22 06:22 UTC, Matt
Details
New version (1.59 KB, patch)
2007-07-22 07:40 UTC, Matt
Details

Description Matt 2007-07-22 05:28:48 UTC
Created attachment 3936 [details]
The patch

This is a patch to change the "User (contribs)" text in the block log to "User (Talk | contribs)". It's mainly a modification of LogPage.php.
Comment 1 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-07-22 05:37:17 UTC
( "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?
Comment 2 Matt 2007-07-22 06:16:12 UTC
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.
Comment 3 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-07-22 06:20:26 UTC
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.
Comment 4 Matt 2007-07-22 06:22:37 UTC
Created attachment 3937 [details]
Patch v2

This is a new patch, using a more-localised format. The | still isnt localised, but it isnt in many places.
Comment 5 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-07-22 06:40:56 UTC
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).
Comment 6 Matt 2007-07-22 06:43:09 UTC
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. 
Comment 7 Matt 2007-07-22 07:40:28 UTC
Created attachment 3938 [details]
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.
Comment 8 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-07-22 20:05:16 UTC
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.
Comment 9 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-07-22 20:41:49 UTC
(see also bug 10663)
Comment 10 Matt 2007-07-23 07:59:45 UTC
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.
Comment 11 Rob Church 2007-07-23 08:03:34 UTC
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.
Comment 12 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-07-23 18:13:20 UTC
(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.
Comment 13 Matt 2007-07-24 04:52:53 UTC
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?
Comment 14 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-07-24 18:27:53 UTC
For now let's just stick with the current issue, and worry about customizability for *all* userToolLinks uses later.
Comment 15 Rob Church 2007-08-06 07:10:21 UTC
Fixed in r24612.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links