r79358 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r79357‎ | r79358 | r79359 >
Date:17:48, 31 December 2010
Author:dantman
Status:ok (Comments)
Tags:
Comment:
Add SkinTemplateNavigation::SpecialPage and SkinTemplateNavigation::Universal hooks to match the content_actions based hooks that vector based hooks are missing.
Modified paths:
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/skins/Vector.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/Vector.php
@@ -305,6 +305,8 @@
306306 'text' => wfMsg( 'nstab-special' ),
307307 'href' => $wgRequest->getRequestURL()
308308 );
 309+ // Equiv to SkinTemplateBuildContentActionUrlsAfterSpecialPage
 310+ wfRunHooks( 'SkinTemplateNavigation::SpecialPage', array( &$this, &$links ) );
309311 }
310312
311313 // Gets list of language variants
@@ -331,6 +333,9 @@
332334 }
333335 }
334336
 337+ // Equiv to SkinTemplateContentActions
 338+ wfRunHooks( 'SkinTemplateNavigation::Universal', array( &$this, &$links ) );
 339+
335340 wfProfileOut( __METHOD__ );
336341
337342 return $links;
Index: trunk/phase3/docs/hooks.txt
@@ -1499,7 +1499,10 @@
15001500 [See http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/examples/Content_action.php
15011501 for an example]
15021502
1503 -'SkinTemplateNavigation': Alter the structured navigation links in SkinTemplates
 1503+Alter the structured navigation links in SkinTemplates, there are three of these hooks called in different spots.
 1504+'SkinTemplateNavigation': Called on content pages before variants have been added
 1505+'SkinTemplateNavigation::SpecialPage': Called on special pages before variands have been added
 1506+'SkinTemplateNavigation::Universal': Called on both content and special pages after variants have been added
15041507 &$sktemplate: SkinTemplate object
15051508 &$links: Structured navigation links
15061509 This is used to alter the navigation for skins which use buildNavigationUrls such as Vector.

Follow-up revisions

RevisionCommit summaryAuthorDate
r797191.17: MFT r78078, r78285, r78787, r79246, r79358, r79480, r79481, r79491, r79...catrope14:15, 6 January 2011

Comments

#Comment by Dantman (talk | contribs)   14:21, 4 January 2011

This would probably be good to backport to 1.17 since it's been missing for awhile and some extensions need them.

#Comment by Catrope (talk | contribs)   22:27, 5 January 2011

Untagging 1.17. None of the 1.17 extensions would use this as the hook was introduced after the branch point.

At this time I'd like to only merge fixes into 1.17, with a minimum of new features (the bar is a bit lower for ResourceLoader and new-installer as they're pretty much the flagship features in 1.17).

#Comment by Dantman (talk | contribs)   22:32, 5 January 2011

content_actions has 3 hooks with varying purpose. Vector has a parallel set of code to generate tabs, but it only has one parallel hook. These two hooks are the other two parallel hooks that are missing in vector.

Without these hooks, extensions which require the use of either of the other hooks will be left in a situation where the tabs they add to the page do not function in Vector.

It would be ideal for us to not leave this bug inside MediaWiki for yet another release, just for the stake of not backporting what is nothing but the addition of two hooks with known behavior.

#Comment by Catrope (talk | contribs)   22:35, 5 January 2011

Right, so it is a bug, not more hook bloat. Alright then, readding 1.17

Status & tagging log