r102704 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r102703‎ | r102704 | r102705 >
Date:22:27, 10 November 2011
Author:yaron
Status:deferred (Comments)
Tags:
Comment:
Fixed handling of forms whose name contains a slash, via new helper function, getFormEditURL()
Modified paths:
  • /trunk/extensions/SemanticForms/specials/SF_FormStart.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SemanticForms/specials/SF_FormStart.php
——@@ -120,6 +120,19 @@
121121 $wgOut->addHTML( $text );
122122 }
123123
 124+ /**
 125+ * Helper function - returns a URL that includes Special:FormEdit.
 126+ */
 127+ static function getFormEditURL( $formName, $targetTitle) {
 128+ $fe = SpecialPage::getPage( 'FormEdit' );
 129+ $targetName = SFUtils::titleURLString( $targetTitle );
 130+ // Special handling for forms whose name contains a slash.
 131+ if ( strpos( $formName, '/' ) !== false ) {
 132+ return $fe->getTitle()->getFullURL( array( 'form' => $formName, 'target' => $targetName ) );
 133+ }
 134+ return $fe->getTitle()->getFullURL() . "/" . $formName . "/" . $targetName;
 135+ }
 136+
124137 function doRedirect( $form_name, $page_name, $params ) {
125138 global $wgOut;
126139
——@@ -148,12 +161,10 @@
149162 if ( $form_name == $default_form_name ) {
150163 $redirect_url = $page_title->getLocalURL( 'action=formedit' );
151164 } else {
152 - $fe = SpecialPage::getPage( 'FormEdit' );
153 - $redirect_url = $fe->getTitle()->getFullURL() . "/" . $form_name . "/" . SFUtils::titleURLString( $page_title );
 165+ $redirect_url = self::getFormEditURL( $form_name, $page_title );
154166 }
155167 } else {
156 - $fe = SpecialPage::getPage( 'FormEdit' );
157 - $redirect_url = $fe->getTitle()->getFullURL() . "/" . $form_name . "/" . SFUtils::titleURLString( $page_title );
 168+ $redirect_url = self::getFormEditURL( $form_name, $page_title );
158169 // Of all the request values, send on to 'FormEdit'
159170 // only 'preload' and specific form fields - we can
160171 // identify the latter because they show up as arrays.

Follow-up revisions

RevisionCommit summaryAuthorDate
r104040Fix for r102704 - fixed getFormEditURL(), based on code suggestions from Nike...yaron14:48, 23 November 2011
r104839Fixes for r102704 and r102365 - improved handling for non-Latin characters in...yaron06:10, 1 December 2011

Comments

#Comment by Nikerabbit (talk | contribs)   08:46, 11 November 2011

I would perhaps do: 

$fe = SpecialPage::getPage( 'FormEdit' );
if ( strpos( $formName, '/' ) !== false ) {
	return $fe->getTitle()->getFullURL( array( 'form' => $formName, 'target' => $targetName ) );
}
return $fe->getTitle( "$formName/$targetName" )->getFullURL();

Any reason to use getFullURL over getLocalURL here?

#Comment by Yaron Koren (talk | contribs)   14:54, 23 November 2011

Thanks for these suggestions! I wish I had incorporated them sooner, because it turned out that there was a bug in my original code that these changes fixed. I can't explain it, but as long as it works, I'm not complaining.