r70106 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r70105‎ | r70106 | r70107 >
Date:20:00, 28 July 2010
Author:mah
Status:reverted (Comments)
Tags:
Comment:
Since script ordering matters, follow up r70100 with a way to load js files after the LQT support has been loaded.
Modified paths:
  • /trunk/extensions/LiquidThreads/classes/View.php (modified) (history)
  • /trunk/extensions/LiquidThreads/pages/NewUserMessagesView.php (modified) (history)

Diff [purge]

Index: trunk/extensions/LiquidThreads/classes/View.php
——@@ -25,7 +25,7 @@
2626
2727 protected $sort_order = LQT_NEWEST_CHANGES;
2828
29 - static $stylesAndScriptsDone = false;
 29+ static $stylesAndScriptsDone = array();
3030
3131 function __construct( &$output, &$article, &$title, &$user, &$request ) {
3232 $this->article = $article;
——@@ -1182,8 +1182,19 @@
11831183 * Output methods *
11841184 *************************/
11851185
1186 - static function addJSandCSS() {
1187 - if ( self::$stylesAndScriptsDone ) {
 1186+ static function sendMoreScripts( $scripts = array() ) {
 1187+ global $wgOut;
 1188+ foreach( $scripts as $file ) {
 1189+ if( !in_array( $file, self::$stylesAndScriptsDone ) ) {
 1190+ $wgOut->addScriptFile( $file );
 1191+ self::$stylesAndScriptsDone[] = $file;
 1192+ }
 1193+ }
 1194+ }
 1195+
 1196+ static function addJSandCSS( $scripts = array() ) {
 1197+ if ( count( self::$stylesAndScriptsDone ) ) {
 1198+ self::sendMoreScripts( $scripts );
11881199 return;
11891200 }
11901201
——@@ -1193,22 +1204,22 @@
11941205
11951206 LqtHooks::$scriptVariables['wgLqtMessages'] = self::exportJSLocalisation();
11961207
 1208+
 1209+ $wgOut->addExtensionStyle( "$wgLiquidThreadsExtensionPath/jquery/jquery-ui-1.7.2.css" );
 1210+ $wgOut->addExtensionStyle( "$wgLiquidThreadsExtensionPath/lqt.css?{$wgStyleVersion}" );
 1211+
 1212+ array_unshift( $scripts, "$wgLiquidThreadsExtensionPath/js/lqt.toolbar.js" );
 1213+ array_unshift( $scripts, "$wgLiquidThreadsExtensionPath/jquery/jquery.autogrow.js" );
 1214+ array_unshift( $scripts, "$wgLiquidThreadsExtensionPath/lqt.js" );
 1215+
11971216 if ( method_exists( $wgOut, 'includeJQuery' ) ) {
11981217 $wgOut->includeJQuery();
1199 - $wgOut->addScriptFile( "$wgLiquidThreadsExtensionPath/jquery/plugins.js" );
 1218+ array_unshift( $scripts, "$wgLiquidThreadsExtensionPath/jquery/plugins.js" );
12001219 } else {
1201 - $wgOut->addScriptFile( "$wgLiquidThreadsExtensionPath/jquery/js2.combined.js" );
 1220+ array_unshift( $scripts, "$wgLiquidThreadsExtensionPath/jquery/js2.combined.js" );
12021221 }
12031222
1204 - $wgOut->addExtensionStyle( "$wgLiquidThreadsExtensionPath/jquery/jquery-ui-1.7.2.css" );
1205 -
1206 - $wgOut->addScriptFile( "$wgLiquidThreadsExtensionPath/jquery/jquery.autogrow.js" );
1207 -
1208 - $wgOut->addScriptFile( "$wgLiquidThreadsExtensionPath/lqt.js" );
1209 - $wgOut->addScriptFile( "$wgLiquidThreadsExtensionPath/js/lqt.toolbar.js" );
1210 - $wgOut->addExtensionStyle( "$wgLiquidThreadsExtensionPath/lqt.css?{$wgStyleVersion}" );
1211 -
1212 - self::$stylesAndScriptsDone = true;
 1223+ self::sendMoreScripts( $scripts );
12131224 }
12141225
12151226 static function exportJSLocalisation() {
——@@ -1888,7 +1899,8 @@
18891900
18901901 $html = '';
18911902 if ( wfRunHooks( 'EditPageBeforeEditToolbar', array( &$html ) ) ) {
1892 - self::addJSandCSS();
 1903+ $scripts = isset( $options['addScripts'] ) ? $options['addScripts'] : array();
 1904+ self::addJSandCSS( $scripts );
18931905 }
18941906
18951907 $class = $this->threadDivClass( $thread );
Index: trunk/extensions/LiquidThreads/pages/NewUserMessagesView.php
——@@ -85,13 +85,6 @@
8686 function showOnce() {
8787 NewMessages::recacheMessageCount( $this->user->getId() );
8888
89 - static $scriptDone = false;
90 -
91 - if ( !$scriptDone ) {
92 - global $wgOut, $wgLiquidThreadsExtensionPath;
93 - $wgOut->addScriptFile( "$wgLiquidThreadsExtensionPath/newmessages.js" );
94 - }
95 -
9689 $this->user->setNewtalk( false );
9790
9891 if ( $this->methodApplies( 'mark_as_unread' ) ) {
——@@ -207,8 +200,9 @@
208201
209202 $mustShowThreads = $this->targets[$t->id()];
210203
211 - $this->showThread( $t, 1, 1, array( 'mustShowThreads' => $mustShowThreads ) );
212 -
 204+ global $wgLiquidThreadsExtensionPath;
 205+ $this->showThread( $t, 1, 1, array( 'mustShowThreads' => $mustShowThreads,
 206+ 'addScripts' => array( "$wgLiquidThreadsExtensionPath/newmessages.js" ) ) );
213207 $this->output->addHTML( "</td></tr>" );
214208 }
215209

Follow-up revisions

RevisionCommit summaryAuthorDate
r70378Update LiquidThreads alpha to trunk state, with r70100 and r70106 unmerged du...werdna04:57, 3 August 2010
r70424revert r70106 and switch ordering of addJSandCSS and hookmah20:59, 3 August 2010
r70448provide a simpler, faster fix for the problem that r70106 was trying to solve.mah04:30, 4 August 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r70100re-apply r70076, WITH A FIX!mah17:59, 28 July 2010

Comments

#Comment by Werdna (talk | contribs)   04:47, 3 August 2010

Surely it would be easier to just call the hook after calling addJsAndCss().

#Comment by MarkAHershberger (talk | contribs)   20:59, 3 August 2010

Right, thanks. I overthought this.

#Comment by MarkAHershberger (talk | contribs)   04:34, 4 August 2010

Argh... r70448.

The problem was not the hook -- the problem was the "New Messages" special page that was trying add newmessages.js. Without some way to force newmessages.js to appear after lqt.js, you would see js errors ("liquidThreads is not defined") on the Special Page.

(I'm not particularly fond of the static that is used in r70448, but I figure that since it is faster than the above solution, it stays.)

Status & tagging log