r89958 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r89957‎ | r89958 | r89959 >
Date:12:58, 13 June 2011
Author:wikinaut
Status:reverted (Comments)
Tags:
Comment:
fix for bug29200 - Creation of new thread fails silently if no content is entered, but this should be possible. Solution: add automatically thread title as comment to new page
Modified paths:
  • /trunk/extensions/LiquidThreads/api/ApiThreadAction.php (modified) (history)
  • /trunk/extensions/LiquidThreads/i18n/Lqt.i18n.php (modified) (history)

Diff [purge]

Index: trunk/extensions/LiquidThreads/i18n/Lqt.i18n.php
——@@ -295,6 +295,7 @@
296296
297297 // Automatic summaries
298298 'lqt-newpost-summary' => 'New thread: $1',
 299+ 'lqt-newpost-defaultbody-user-submitted-empty-textbody' => "<!-- thread subject $1 -->''Please enter your text for this thread. You should delete this line then.''",
299300 'lqt-reply-summary' => 'Reply to [[$2|$1]]',
300301
301302 // Talk page history
Index: trunk/extensions/LiquidThreads/api/ApiThreadAction.php
——@@ -222,7 +222,7 @@
223223 }
224224
225225 public function actionNewThread( $threads, $params ) {
226 - global $wgUser;
 226+ global $wgUser,$wgOut;
227227
228228 // Validate talkpage parameters
229229 if ( !count( $params['talkpage'] ) ) {
——@@ -262,7 +262,9 @@
263263
264264 // Check for text
265265 if ( empty( $params['text'] ) ) {
266 - $this->dieUsage( 'You must include text in your post', 'no-text' );
 266+ // $this->dieUsage( 'You must include text in your post', 'no-text' );
 267+ // FIXME: add an "edit post" link as second parameter $2 to the text
 268+ $params['text'] = wfMsgForContent( 'lqt-newpost-defaultbody-user-submitted-empty-textbody', $subject ) ;
267269 }
268270 $text = $params['text'];
269271

Follow-up revisions

RevisionCommit summaryAuthorDate
r89967when adding a new thread with an empty text body, a placeholder default text ...wikinaut14:39, 13 June 2011
r97916Revert r89958 and replace it with a proper solutionwerdna12:57, 23 September 2011

Comments

#Comment by Werdna (talk | contribs)   13:10, 13 June 2011

This won't work for non-AJAX submissions. I'd much rather fail outright than put an HTML comment in.

#Comment by Wikinaut (talk | contribs)   13:14, 13 June 2011

I do not think that i fails with non-Ajax. Please try it.

#Comment by Wikinaut (talk | contribs)   13:14, 13 June 2011

s/i fails/it fails/

#Comment by Wikinaut (talk | contribs)   13:15, 13 June 2011

current lqt without ajax - this fails. Lqt is broken in many things, and help is far away

#Comment by Werdna (talk | contribs)   13:19, 13 June 2011

The current version of LiquidThreads is broken in many respects. This is by far not the most serious bug. As I've said to you before, I think the appropriate way forward is a complete rewrite, which is under way (but it's slow progress — it's not one of the Foundation's priorities and I end up assigned to other projects).

#Comment by Wikinaut (talk | contribs)   13:50, 13 June 2011

I do not see _any_ progress in lqt rewrite, because not a single code line has been published yet. However, when I see the many implementations of (current) Lqt, all these users are happy about my tiny bug-fixes See for example the "drag to new location" does not work problem (fixed since last week).

This one here was urgently needed, as there was not even an error message (when the new thread was submitted without any body).

#Comment by Nikerabbit (talk | contribs)   13:52, 13 June 2011

http://www.mediawiki.org/w/index.php?path=branches%2Flqt-updates%2F&title=Special%3ACode%2FMediaWiki%2Fpath

PS: can the old LQTRewrite branch be deleted?

#Comment by Wikinaut (talk | contribs)   18:13, 23 September 2011

$this->dieUsage( 'You must include text in your post', 'no-text' );

is not working, basically. See reopened bug29200 - please, we need someone else to fix that, I cannot find the reason for the bug. It is somewhere in the exception handling, which is extremely difficult to track (Brion tried this already, without immediate results). Lqt needs a general and common approach, initiative. The Lqt3.0 team ? When can we see some output ?

#Comment by Werdna (talk | contribs)   00:10, 24 September 2011

Did you read the commit I made?

+ if ( text.trim().length == 0 ) { + alert(mediaWiki.msg('lqt-empty-text')); + return; + }

That will fix the bug properly.

#Comment by Wikinaut (talk | contribs)   06:25, 24 September 2011

It's the exception handling which needs help. May be, that it worked in former times, but not now.

Lqt API http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/LiquidThreads/api/ApiThreadAction.php has already tons of broken code à la 

381 	// Check for text
382 	if ( empty( $params['text'] ) ) {
383 	$this->dieUsage( 'You must include text in your post', 'no-text' );
384 	}
385 	$text = $params['text'];

which is not working, this needs to be fixed.

#Comment by Werdna (talk | contribs)   06:50, 24 September 2011

Why don't you think it's working? What do you mean by "it's not working"? Is it spending too much time on Facebook?

#Comment by Wikinaut (talk | contribs)   06:59, 24 September 2011

You presented code which uses alert() instead of using the integrated exception handler.

#Comment by Werdna (talk | contribs)   07:19, 24 September 2011

Right, it's not the most elegant possible solution – at present the UI handles errors reported by the API by posting the form manually and showing the errors using the standard HTTP request.

However, the code I committed does deal with the immediate bug (it works). Proper UI handling for the range of errors that may be reported by the API is planned for after the LiquidThreads backend rewrite.

Status & tagging log