Last modified: 2011-03-13 17:45:57 UTC
SkinTemplate.php's buildContentActionUrls ignores protection for article and talk pages. It should be updated to deal with today's $wgNamespaceProtection etc. You see, users are given (red)links to go edit Article and Talk pages that they cannot possibly edit. Users should not be even given any link at all, for pages that don't exist and they have no rights to create. In SkinTemplate.php let's take a look at this part of function buildContentActionUrls: if( $this->iscontent ) { $subjpage = $this->mTitle->getSubjectPage(); $talkpage = $this->mTitle->getTalkPage(); $nskey = $this->mTitle->getNamespaceKey(); $content_actions[$nskey] = $this->tabAction( $subjpage,$nskey,!$this->mTitle->isTalkPage() && !$prevent_active_tabs,'',true); $content_actions['talk'] = $this->tabAction($talkpage,'talk', $this->mTitle->isTalkPage() && !$prevent_active_tabs,'',true); if($this->mTitle->quickUserCan('edit')&&($this->mTitle->exists()||$this->mTitle->quickUserCan('create'))){ Well, the two $content_actions unfortunately come BEFORE the quickUserCan tests. This means the user could get (red)links to pages he cannot possibly create. Also he will get redlinks to pages he cannot possibly edit, if they already exist. Case in point: In LocalSettings.php we set $wgNamespaceProtection[NS_CATEGORY]=$wgNamespaceProtection[NS_CATEGORY_TALK]=array('editinterface');} So, e.g., browsing a category page (with members, but no content), he sees two redlinks, one to the category page itself, and one to the category page's talk page (which doesn't exist.) Well, he shouldn't see any link at all to the talk page. And he should see a blue link to the category page itself. And if he browses to a URL which doesn't exist, no talk page too, and he has no rights... he should see no tabs at all.
> come BEFORE the Actually it is not as simple as placing them AFTER. They need their own tests. Also one might say 'well, clean it up via a hook'. Well, that would currently be very inefficient. Also, it is a bug. Not just something one should just use a hook to workaround.
(I really am using 1.15 SVN. The code fragment above looks different because I took out the whitespace.)
(In reply to comment #0) > SkinTemplate.php's buildContentActionUrls ignores protection for > article and talk pages. > > It should be updated to deal with today's $wgNamespaceProtection etc. > It does deal with them by showing "view source" tabs instead of edit tabs above a protected page, > You see, users are given (red)links to go edit Article and Talk pages > that they cannot possibly edit. > So? This happens for redlinks inside articles too. > Users should not be even given any link at all, for pages that don't > exist and they have no rights to create. > That means not showing the tabs at all (a suggestion you repeat below), which is pretty confusing, especially for the article/talk tab. Recommend WONTFIX: hiding these tabs doesn't accomplish much and will only confuse people.
> It does deal with them by showing "view source" tabs instead of edit tabs above > a protected page, Let's talk about when the talk page doesn't exist. You give them a red link inviting them to go edit a talk page that doesn't exist and they have no permission to edit. > So? This happens for redlinks inside articles too. That can be blamed on authors. But we're talking about the MediaWiki provided links at the tops of pages here. You don't give them a link to delete a page that they don't have any permission to delete, so why then give them a talk link that they will totally regret clicking? > That means not showing the tabs at all (a suggestion you repeat below), which > is pretty confusing, especially for the article/talk tab. At least don't give them a "bait and switch" talk tab. It is rare that a talk page would exist, but not its article. So it is safe to check if "talk page exists or we have permission to create it" before making a talk link, (red or blue,) ... please.
(In reply to comment #4) > > It does deal with them by showing "view source" tabs instead of edit tabs above > > a protected page, > > Let's talk about when the talk page doesn't exist. > You give them a red link inviting them to go edit a talk page that > doesn't exist and they have no permission to edit. > That'd confuse people, because they expect the talk tab to be there. When the talk tab is just gone, it's far from obvious that that's because the talk page doesn't exist and can't be created by the user. > You don't give them a link to delete a page that they don't have any > permission to delete, True, delete tabs are hidden for users without delete rights, and move tabs are hidden on move-protected pages. Still, these are different: move and delete are actions, the article/talk tabs are for navigation. > so why then give them a talk link that they will > totally regret clicking? > "totally regret" is kind of hyperbolic: seeing a page that says you're not allowed to create the talk page and then clicking Back to get back to where you were is not a big deal. Also, I believe the principle of least surprise applies here: showing a link that leads to an error message is less confusing than not showing the link at all when people expect it to be there. > > That means not showing the tabs at all (a suggestion you repeat below), which > > is pretty confusing, especially for the article/talk tab. > > At least don't give them a "bait and switch" talk tab. > Then what do you suggest? You either show the tab or you don't, and not showing it is the more confusing option. Closing this bug as WONTFIX, since it requests something that'll harm rather than improve usability.
Ah: a greyed out unclickable talk link! But wait, that would make the user ever more furious: "why won't you let me even read it!?" Also greyed out unclickable links would require big code enhancement. Hmmm. Wait, I have made a workaround via a hook to undo the ugliness after it has been made: function JidanniLessRedContentActions($sktemplate,$content_actions){ if('new'==$content_actions['talk']['class']&&!$sktemplate->mTitle->quickUserCan('createtalk')){ unset($content_actions['talk']);} if('selected new'==$content_actions['nstab-category']['class']){ $content_actions['nstab-category']['class']='selected';} return true;} $wgHooks['SkinTemplateTabs'][]='JidanniLessRedContentActions'; I'll use my hook for now...
Well now here we are in 2010 and Vector is the default skin in CVS which is what my wikis use. And now most of my workarounds need to researched and rewritten for Vector, to prevent my website from looking ugly which it has now become again.
(In reply to comment #6) > function JidanniLessRedContentActions($sktemplate,$content_actions) OK, I finally figured out how to do that in Vector. For the record here it is: function JidanniLessRedContentActionsVectorTypeSkins($sktemplate,$links){ if(isset($links['namespaces']['talk']['class'])&& 'new'==$links['namespaces']['talk']['class']&& !$sktemplate->mTitle->quickUserCan('createtalk')){ unset($links['namespaces']['talk']); if(isset($links['actions']['watch'])){unset($links['actions']['watch']);};}; if(isset($links['namespaces']['category_talk']['class'])&& 'new'==$links['namespaces']['category_talk']['class']&& !$sktemplate->mTitle->quickUserCan('createtalk')){ unset($links['namespaces']['category_talk']); if(isset($links['actions']['watch'])){unset($links['actions']['watch']);};}; if(isset($links['namespaces']['category']['class'])&& 'selected new'==$links['namespaces']['category']['class']){ $links['namespaces']['category']['class']='selected';} return true;} $wgHooks['SkinTemplateNavigation'][]='JidanniLessRedContentActionsVectorTypeSkins';} Odd that when one uses vector, both hooks mention above get run...
(In reply to comment #8) even better, just for the record, in case anybody needs it: function JidanniLessRedContentActionsVectorTypeSkins($sktemplate,$links){ if(isset($links['namespaces'])&& is_array($links['namespaces'])&& !$sktemplate->mTitle->quickUserCan('createtalk')){ foreach(array_keys($links['namespaces']) as $ns){ if(strpos($ns,'talk')!==false){ if(isset($links['namespaces'][$ns]['class'])&& 'new'==$links['namespaces'][$ns]['class']){ unset($links['namespaces'][$ns]);}}} if(isset($links['actions']['watch'])){unset($links['actions']['watch']);}} if(isset($links['namespaces']['category']['class'])&& 'selected new'==$links['namespaces']['category']['class']){ $links['namespaces']['category']['class']='selected';} return true;} $wgHooks['SkinTemplateNavigation'][]='JidanniLessRedContentActionsVectorTypeSkins';
Changing all WONTFIX high priority bugs to lowest priority (no mail should be generated since I turned it off for this.)
