r66835 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r66834‎ | r66835 | r66836 >
Date:16:38, 24 May 2010
Author:platonides
Status:reverted (Comments)
Tags:
Comment:
Follow up r62958. addToSidebar() was added in r59215, we still can redesign it and avoid legacy functions.
Modified paths:
  • /trunk/extensions/DynamicSidebar/DynamicSidebar.body.php (modified) (history)
  • /trunk/extensions/GroupsSidebar/GroupsSidebar.body.php (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Skin.php
——@@ -2104,7 +2104,7 @@
21052105 }
21062106
21072107 $bar = array();
2108 - $this->addToSidebar( $bar, 'sidebar' );
 2108+ $this->addToSidebar( $bar, wfMsgForContentNoTrans( 'sidebar' ) );
21092109
21102110 wfRunHooks( 'SkinBuildSidebar', array( $this, &$bar ) );
21112111 if ( $wgEnableSidebarCache ) {
——@@ -2113,26 +2113,16 @@
21142114 wfProfileOut( __METHOD__ );
21152115 return $bar;
21162116 }
2117 - /**
2118 - * Add content from a sidebar system message
2119 - * Currently only used for MediaWiki:Sidebar (but may be used by Extensions)
2120 - *
2121 - * This is just a wrapper around addToSidebarPlain() for backwards compatibility
2122 - *
2123 - * @param &$bar array
2124 - * @param $message String
2125 - */
2126 - function addToSidebar( &$bar, $message ) {
2127 - $this->addToSidebarPlain( $bar, wfMsgForContent( $message ) );
2128 - }
21292117
21302118 /**
2131 - * Add content from plain text
2132 - * @since 1.17
 2119+ * Add content to the sidebar from text
 2120+ * @since 1.16
21332121 * @param &$bar array
21342122 * @param $text string
 2123+ *
 2124+ * @return array
21352125 */
2136 - function addToSidebarPlain( &$bar, $text ) {
 2126+ function addToSidebar( &$bar, $text ) {
21372127 $lines = explode( "\n", $text );
21382128 $heading = '';
21392129 foreach( $lines as $line ) {
Index: trunk/extensions/DynamicSidebar/DynamicSidebar.body.php
——@@ -32,15 +32,15 @@
3333 $catSB = array();
3434 if ( $wgDynamicSidebarUseGroups && isset( $sidebar['GROUP-SIDEBAR'] ) ) {
3535 self::printDebug( "Using group sidebar" );
36 - $skin->addToSidebarPlain( $groupSB, self::doGroupSidebar() );
 36+ $skin->addToSidebar( $groupSB, self::doGroupSidebar() );
3737 }
3838 if ( $wgDynamicSidebarUseUserpages && isset( $sidebar['USER-SIDEBAR'] ) ) {
3939 self::printDebug( "Using user sidebar" );
40 - $skin->addToSidebarPlain( $userSB, self::doUserSidebar() );
 40+ $skin->addToSidebar( $userSB, self::doUserSidebar() );
4141 }
4242 if ( $wgDynamicSidebarUseCategories && isset( $sidebar['CATEGORY-SIDEBAR'] ) ) {
4343 self::printDebug( "Using category sidebar" );
44 - $skin->addToSidebarPlain( $catSB, self::doCategorySidebar() );
 44+ $skin->addToSidebar( $catSB, self::doCategorySidebar() );
4545 }
4646
4747 $sidebar_copy = array();
Index: trunk/extensions/GroupsSidebar/GroupsSidebar.body.php
——@@ -11,7 +11,7 @@
1212 foreach ( $wgGroupsSidebar as $group => $sectiontitle ) {
1313 if (in_array($group, $wgUser->getEffectiveGroups())) {
1414 $message = 'sidebar-'.$sectiontitle;
15 - $skin->addToSidebar( &$bar, $message );
 15+ $skin->addToSidebar( $bar, wfMsgForContentNoTrans( $message ) );
1616 }
1717 }
1818 return true;

Follow-up revisions

RevisionCommit summaryAuthorDate
r66836Merge r66835platonides16:44, 24 May 2010
r66846Follow up r66835. Some people use direct urls in MediaWiki:Sidebar instead of...platonides20:01, 24 May 2010
r68707Revert r66835, r66846: it's too late to break interfaces in 1.16.tstarling06:39, 29 June 2010
r82452* Use $this->mTitle instead of $wgTitle...ialex13:39, 19 February 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r59215Make adding sidebar elements easier for extensions (e.g. Extension:GroupsSide...churchofemacs21:06, 18 November 2009
r62958Rename Skin::addToSidebar() to Skin::addToSidebarPlain() and have it take pla...catrope15:58, 25 February 2010

Comments

#Comment by Nikerabbit (talk | contribs)   20:00, 24 May 2010

Breaks part of the sidebar in twn. Input: 

* recentchanges
** {{FULLURL:Special:RecentChanges|translations=only&trailer=/{{UILANGCODE}}}} | bw-sidebar-changes-lang
** {{FULLURL:Special:RecentChanges|translations=only}}                         | bw-sidebar-changes-lang-all
** {{FULLURL:Special:RecentChanges|translations=filter}}                       | bw-sidebar-changes-wiki
#Comment by Nikerabbit (talk | contribs)   20:04, 24 May 2010

Fixed by r66846.

#Comment by Tim Starling (talk | contribs)   06:30, 29 June 2010

It's too late, 1.16 is released already. Further minor releases are for bug fixes only. Extension authors have a right to expect interface stability across minor releases, even if those minor releases are beta releases.

#Comment by Platonides (talk | contribs)   18:30, 30 June 2010

r59215 was done just for Extension:GroupsSidebar (DynamicSidebar was added later and is a supergroup). I could agree if it was something likely to have been widely used but I don't see such evidence here. If we are going to commit to such level of compatibility, why don't directly release it?

Status & tagging log