r95631 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r95630‎ | r95631 | r95632 >
Date:17:41, 28 August 2011
Author:hartman
Status:reverted (Comments)
Tags:
Comment:
Follow up to r79941

To me it doesn't seem like there is a reason that the modules should be publicized in the api.
The modules need to be loaded locally, so the dependancy is already on local loading.

Adding a new ext.liquidThreads.edit module to do the loading of the edit mode of lqt as well as dependancy loading.
Modified paths:
  • /trunk/extensions/LiquidThreads/LiquidThreads.php (modified) (history)
  • /trunk/extensions/LiquidThreads/api/ApiThreadAction.php (modified) (history)
  • /trunk/extensions/LiquidThreads/lqt.js (modified) (history)

Diff [purge]

Index: trunk/extensions/LiquidThreads/LiquidThreads.php
——@@ -70,7 +70,7 @@
7171
7272 $wgResourceModules['ext.liquidThreads'] = $lqtResourceTemplate + array(
7373 'styles' => array( 'lqt.css', 'jquery/jquery.thread_collapse.css', 'lqt.dialogs.css' ),
74 - 'scripts' => array( 'lqt.js', 'js/lqt.toolbar.js', 'jquery/jquery.thread_collapse.js', 'jquery/jquery.autogrow.js' ),
 74+ 'scripts' => array( 'lqt.js', 'jquery/jquery.thread_collapse.js', 'jquery/jquery.autogrow.js' ),
7575 'dependencies' => array( 'jquery.ui.dialog', 'jquery.ui.droppable' ),
7676 'messages' => $lqtMessages
7777 );
——@@ -80,6 +80,13 @@
8181 'dependencies' => array( 'ext.liquidThreads' )
8282 );
8383
 84+$wgResourceModules['ext.liquidThreads.edit'] = $lqtResourceTemplate + array(
 85+ 'scripts' => array( 'js/lqt.toolbar.js' ),
 86+ 'dependancies' => array( 'ext.liquidThreads', 'ext.wikiEditor', 'ext.wikiEditor.toolbar.i18n',
 87+ 'jquery.wikiEditor.toolbar', 'jquery.wikiEditor.dialogs',
 88+ 'query.async', 'jquery.cookie')
 89+);
 90+
8491 // Hooks
8592 // Parser Function Setup
8693 $wgHooks['ParserFirstCallInit'][] = 'LqtHooks::onParserFirstCallInit';
Index: trunk/extensions/LiquidThreads/api/ApiThreadAction.php
——@@ -821,11 +821,6 @@
822822
823823 $result = array( 'inlineeditform' => array( 'html' => $output ) );
824824
825 - /* FIXME
826 - $result['resources'] = LqtView::getJSandCSS();
827 - $result['resources']['messages'] = LqtView::exportJSLocalisation();
828 - */
829 -
830825 $this->getResult()->addValue( null, 'threadaction', $result );
831826 }
832827
Index: trunk/extensions/LiquidThreads/lqt.js
——@@ -228,10 +228,7 @@
229229 liquidThreads.loadInlineEditForm( params, container,
230230 function() {
231231 if ( typeof mediaWiki.loader != 'undefined' && mediaWiki.loader ) {
232 - mediaWiki.loader.using(
233 - [ 'ext.wikiEditor', 'ext.wikiEditor.toolbar.i18n',
234 - 'jquery.wikiEditor.toolbar', 'jquery.wikiEditor.dialogs',
235 - 'jquery.async', 'jquery.cookie' ],
 232+ mediaWiki.loader.using( 'ext.liquidThreads.edit',
236233 finishSetup );
237234 } else {
238235 finishSetup();

Follow-up revisions

RevisionCommit summaryAuthorDate
r96172Correct spelling of dependencies...hartman07:09, 3 September 2011
r96278Revert r95631, should be enough to fix the issues with LQT edit form. TheDJ a...catrope13:43, 5 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r79941Follow up to r79905. Disabling resource inclusion of LQT api. Don't know how ...hartman17:18, 10 January 2011

Comments

#Comment by Catrope (talk | contribs)   14:27, 31 August 2011 
+	'dependancies' => array( 'ext.liquidThreads', 'ext.wikiEditor', 'ext.wikiEditor.toolbar.i18n',

How did this rev even work for you locally if you misspelled 'dependencies'?

Looks fine otherwise.

#Comment by MaxSem (talk | contribs)   13:57, 4 September 2011

This introduces an explicit dependency on WikiEditor. Previously, its modules quietly failed to load. Now, modules depending on WikiEditor simply explode, see r96206.

#Comment by TheDJ (talk | contribs)   07:19, 5 September 2011

I would appreciate it if someone can undo all my RL changes to LQT. I don't have the time to work on this, and the fixme's emails are annoying.

#Comment by Catrope (talk | contribs)   07:22, 5 September 2011

I'm on it.

#Comment by Catrope (talk | contribs)   13:47, 5 September 2011

Ended up just reverting this revision; I looked at pulling out all RL-related stuff in LQT but that would be too nightmarish. Not setting r79941 back to fixme since things apparently worked fine, it just wasn't done nicely.

#Comment by Catrope (talk | contribs)   13:27, 5 September 2011 
+				'query.async', 'jquery.cookie')

That was another misspelling, and probably the real reason LQT exploded on TranslateWiki.

Status & tagging log