r95327 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r95326‎ | r95327 | r95328 >
Date:19:56, 23 August 2011
Author:johnduhart
Status:ok (Comments)
Tags:
Comment:
(bug 30335) Fix for HTMLForms using GET breaking when non-friendly URLs are used
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/HTMLForm.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
——@@ -68,6 +68,8 @@
6969 * (bug 11374) Improved diff readability for colorblind people.
7070 * (bug 26486) ResourceLoader modules with paths to nonexistent files cause PHP
7171 warnings/notices to be thrown.
 72+* (bug 30335) Fix for HTMLForms using GET breaking when non-friendly URLs are
 73+ used
7274
7375 === API changes in 1.19 ===
7476 * (bug 19838) siprop=interwikimap can now use the interwiki cache.
Index: trunk/phase3/includes/HTMLForm.php
——@@ -430,12 +430,18 @@
431431 * @return String HTML.
432432 */
433433 function getHiddenFields() {
 434+ global $wgUsePathInfo;
 435+
434436 $html = '';
435437 if( $this->getMethod() == 'post' ){
436438 $html .= Html::hidden( 'wpEditToken', $this->getUser()->editToken(), array( 'id' => 'wpEditToken' ) ) . "\n";
437439 $html .= Html::hidden( 'title', $this->getTitle()->getPrefixedText() ) . "\n";
438440 }
439441
 442+ if ( !$wgUsePathInfo && $this->getMethod() == 'get' ) {
 443+ $html .= Html::hidden( 'title', $this->getTitle()->getPrefixedText() ) . "\n";
 444+ }
 445+
440446 foreach ( $this->mHiddenFields as $data ) {
441447 list( $value, $attribs ) = $data;
442448 $html .= Html::hidden( $attribs['name'], $value, $attribs ) . "\n";

Follow-up revisions

RevisionCommit summaryAuthorDate
r95632MFT to REL1_18...hashar17:57, 28 August 2011

Comments

#Comment by Bawolff (talk | contribs)   03:15, 25 August 2011

I'm not sure if this is the right way to do this - perhaps checking $wgRequest->getVal( 'title' ) to see if the page was requested with a title parameter would be a better solution.

#Comment by Catrope (talk | contribs)   13:38, 25 August 2011

I was thinking maybe we should just set title= unconditionally.

#Comment by Johnduhart (talk | contribs)   17:37, 25 August 2011

Sure that's a solution but I was hoping we were shooting for making URLs cleaner by allowing paramters to be set after the Title like /wiki/Special:BlockList?wpTarget=

#Comment by Bawolff (talk | contribs)   03:18, 26 August 2011

Do we know for sure that query parameters always work with short urls? (I'm remembering the warning on the docs of Linker::link about 'forcearticlepath' having compatibility issues on some setups, but I'm not sure if that warning was actually true)

#Comment by Johnduhart (talk | contribs)   17:35, 25 August 2011

$wgRequest->getVal( 'title' ) is actually set no matter what with a call to $wgRequest->interpolateTitle() in Setup.php on line 436.

Status & tagging log