Last modified: 2008-11-04 01:54:07 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 T17868, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 15868 - [PATCH] Nostalgia skin has no user pages links
[PATCH] Nostalgia skin has no user pages links
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Interface (Other open bugs)
1.14.x
All All
: Normal normal (vote)
: ---
Assigned To: Nobody - You can work on this!
: easy, patch, patch-need-review
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-06 16:16 UTC by NSK Nikolaos S. Karastathis
Modified: 2008-11-04 01:54 UTC (History)
3 users (show)

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


Attachments
Patch that provides user pages links for the Nostalgia skin, tested and works ok (915 bytes, patch)
2008-10-07 08:58 UTC, NSK Nikolaos S. Karastathis
Details
Updated patch to take care of Comment #2 (1.64 KB, patch)
2008-11-04 01:32 UTC, NSK Nikolaos S. Karastathis
Details

Description NSK Nikolaos S. Karastathis 2008-10-06 16:16:08 UTC
MediaWiki ships with a number of skins, one of which is Nostalgia, and is also available at Wikipedia.  However, this skin lacks extremely important features that are normally found in other skins, including Wikipedia's default (MonoBook), one of which is the provision of links to the user pages of the user currently logged in.

Whereas the other skins provide a comfortable link to one's own user page, users of Nostalgia have to live without this.

While to some it may feel normal to have such great differences in usability across skins, I consider this a bug, since Nostalgia is part of MediaWiki releases as well as available on Wikipedia, and therefore a user would expect it to provide the same basic functionality as other skins, just with a different layout.
Comment 1 NSK Nikolaos S. Karastathis 2008-10-07 08:58:47 UTC
Created attachment 5397 [details]
Patch that provides user pages links for the Nostalgia skin, tested and works ok

I fixed this bug and I provide you with the patch, hopping to see it being adopted in the svn repository and the next release :)

I tested my patch and it works perfectly on my test wiki using MediaWiki 1.14alpha (r41797).

What this patch does, exactly:
The Nostalgia skin shipped with MediaWiki and available on Wikipedia.org cannot display the user pages, so what the user sees is this: Main Page | Recent changes | Edit this page | Page history | Log out | 

My patch changes the above links line to this, when the user is logged in: Main Page | Recent changes | Edit this page | Page history | My page | My talk | My watchlist | My contributions | Preferences | Log out | 

Why this patch is important: the user should be able to easily visit their user page, talk page, watchlist, and preferences.  Currently the watchlist and preferences can be visited from the drop down menu.  However, the user has no way to see their user page and talk page, unless they type in the URL or via the search facility, or by clicking a link in another talk page or history page that they have edited in the past (or if someone else refers to them).  That behaviour is very unintuitive and makes life difficult for newbies.  Instead, placing links to one's userpage and talk page in the links line allows newbies to quickly find their user page and other personal stuff.  Furthermore, this capability already exists in all the other skins shipped with MediaWiki and available at Wikipedia, so Nostalgia was at a disadvantage by not including this capability as well.

When the user is logged out, my patch makes no difference.

My patch is based on code from CologneBlue.php

For any questions just ask me.
Comment 2 Aryeh Gregor (not reading bugmail, please e-mail directly) 2008-10-10 13:35:56 UTC
General note: it's considered good style in PHP to use single-quoted strings unless you have a specific reason to use double-quoted strings.  It's a good idea to stick to this for visual consistency.  Also, people might rely on grep "'mypage'" to get all the instances of that message and not incidental other things containing that string, which would fail if someone uses double quotes.

>+			/* show links to user page, user's talk page, etc */
>+			$name = $wgUser->getName();
>+			$tl = $this->makeKnownLinkObj( $wgUser->getTalkPage(),
>+				wfMsg( 'mytalk' ) );

This isn't right: it should be wfMsgHtml( 'mytalk' ), so that the message is HTML-escaped.  This is actually done in MonoBook too, but it's abstracted a bit.

Also, why are you using makeKnownLinkObj()?  There's no guarantee that the page will exist.  Shouldn't it be makeLinkObj() (or actually, link(), which is the cleaner replacement for make*Link*() that I added recently, but never mind that detail)?  That will add the "new" class if the page doesn't exist.

>+			if ( $wgUser->getNewtalk() ) {
>+				$tl .= " *";
>+			}
>+			$s .= $sep . $this->makeKnownLinkObj( $wgUser->getUserPage(),
>+				wfMsg( "mypage" ) )

Same remarks here: needs to be escaped with wfMsgHtml(), and I don't think you want 'known'.

>+				. $sep . $tl

Why even declare $tl if it's only used this one time?  Just do something like

				. $sep . $this->makeKnownLinkObj( $wgUser->getTalkPage(),
					wfMsg( 'mypage' ) );
			if ( $wgUser->getNewtalk() ) {
				$s .= ' *';
			}
			$s .= $sep . $this->specialLink( 'watchlist' )

or whatever.

>+				. $sep . $this->makeKnownLinkObj( SpecialPage::getSafeTitleFor(
>+					"Contributions", $wgUser->getName() ),
>+						wfMsg( "mycontris" ) )

Same two remarks as for the two cases above above -- except that in this case it's a special page and so will always exist, of course.  Although using link() would still be more succinct; the syntax is identical for the two-parameter case, link( Title, raw HTML contents of <a> ).


Now on to the actual effect of the patch: it changes

"Main Page | New changes | Edit this page | Page history | Log out |"

(surely that trailing pipe is a bug?) to

"Main Page | New changes | Edit this page | Page history | My page | My talk | My watched pages | My changes | My settings | Log out |"

This makes the line *way* longer, in fact it now wraps on my wiki.  Note how MonoBook has these links split up into two places: page-specific stuff is in one place, user stuff is in another.  (And both places are set in small type.)  Is this really the best place to put these new links?  I don't use Nostalgia, of course, but it looks significantly more cluttered to me.

I would be willing to commit if those concerns are addressed.  (Although I might not be entirely prompt in getting back to this, depending on what other things I'm doing -- if I'm too tardy you can try finding another dev, of course.)
Comment 3 NSK Nikolaos S. Karastathis 2008-10-10 15:10:33 UTC
See bug #15928 for the trailing |
Comment 4 NSK Nikolaos S. Karastathis 2008-10-10 15:21:55 UTC
Note that in my message I said: "My patch is based on code from CologneBlue.php".  Therefore whatever misperfections the code has it is because all these misperfections were copied delibarately from CologneBlue.php, and if they are to be fixed in Nostalgia then perhaps they should also be fixed in CologneBlue.php.

As for the wraped line, the wrap depends on your screen resolution.  It doesn't wrap in 1920x1200, but it does in 1280x800 (and in fact even the default Wikipedia skin wraps on a PDA or netbook).  I don't consider wrapping a problem, though, and in fact I prefer it to not having userlinks at all or having a second line (thus eating up vertical screen real estate).

Comment 5 NSK Nikolaos S. Karastathis 2008-10-10 15:27:16 UTC
See bug # 15929 for the CologneBlue.php code modernisation issues.
Comment 6 Aryeh Gregor (not reading bugmail, please e-mail directly) 2008-10-10 17:30:58 UTC
(In reply to comment #4)
> Note that in my message I said: "My patch is based on code from
> CologneBlue.php".  Therefore whatever misperfections the code has it is because
> all these misperfections were copied delibarately from CologneBlue.php, and if
> they are to be fixed in Nostalgia then perhaps they should also be fixed in
> CologneBlue.php.

Hmm, okay.  I'll look at that.

> As for the wraped line, the wrap depends on your screen resolution.  It doesn't
> wrap in 1920x1200, but it does in 1280x800 (and in fact even the default
> Wikipedia skin wraps on a PDA or netbook).  I don't consider wrapping a
> problem, though, and in fact I prefer it to not having userlinks at all or
> having a second line (thus eating up vertical screen real estate).

Could I have some input from other Nostalgia users on whether they'd like this change, maybe?  I don't want to have a lynch mob coming after me because they think I've made their skin ugly (it's happened to me before . . .).
Comment 7 NSK Nikolaos S. Karastathis 2008-10-10 22:06:03 UTC
To have input from other Nostalgia users, we should first find these users, and this is possible only if they exist, but I am not sure whether there are other old-fashioned nostalgic people apart from me who have set Nostalgia as their skin of choice.

I had asked someone to help me get statistics to see how many enwiki users have Nostalgia as their user skin.  I will see later whether I got an answer and if so I will post here.

However, I cannot imagine anyone not wanting userlinks in Nostalgia, albeit I can certainly imagine people having different ideas about which specific links should be visible and where.  Perhaps some people wouldn't need the preferences and watchlink links (which are also in the drop down menu).  Maybe some would be happy to only have a link to their user page.  In the patch I just included the links as they are in other skins, to provide the same functionality (links allow one to middleclick to open the link to a new tab or window, but the dropdown menu doesn't allow that, so even duplicating a few links from the dropdown menu is useful as it enables users to have the same middleclick functionality present in the other skins).  I put them in the top because the other skins generally place them somewhere near the top, but maybe if other people want some of them could be moved under the content.  It wouldn't be a bad idea to ask others what they think, if we can find other nostalgic people around, though.
Comment 8 Siebrand Mazeland 2008-11-02 23:25:13 UTC
Please update the patch to resolve the issues raised in comment 2 so it can be applied.
Comment 9 NSK Nikolaos S. Karastathis 2008-11-04 01:32:13 UTC
Created attachment 5501 [details]
Updated patch to take care of Comment #2

This patch should fix the issues raised in Comment #2, thanks for the guidelines.
Comment 10 Charles Melbye 2008-11-04 01:46:39 UTC
I'm working on patching, one sec.
Comment 11 Charles Melbye 2008-11-04 01:54:07 UTC
Fixed in r43182.

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


Navigation
Links