r87808 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r87807‎ | r87808 | r87809 >
Date:05:36, 10 May 2011
Author:bawolff
Status:reverted (Comments)
Tags:
Comment:
Prevent user from trying to move a page to a title longer than 255 bytes.

This just puts a maxlength on the field (+ byte counter js) which is much
nicer and clearer feedback to the user if they went over the limit then:
"The requested page title was invalid, empty, or an incorrectly linked inter-language or inter-wiki title. It may contain one or more characters which cannot be used in titles"
Which doesn't exactly indicate a length error.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMovepage.php (modified) (history)
  • /trunk/phase3/resources/mediawiki.special/mediawiki.special.movePage.js (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
——@@ -27,6 +27,7 @@
2828 used in Tiff files.
2929 * When translcuding a special page, do not let it interpret url parameters.
3030 * (bug 28887) Special page classes are no longer re-used during 1 request.
 31+* New title field of Special:MovePage is now length limited on client side.
3132
3233 === API changes in 1.19 ===
3334
Index: trunk/phase3/includes/specials/SpecialMovepage.php
——@@ -229,7 +229,8 @@
230230 Xml::label( wfMsg( 'newtitle' ), 'wpNewTitle' ) .
231231 "</td>
232232 <td class='mw-input'>" .
233 - Xml::input( 'wpNewTitle', 40, $wgContLang->recodeForEdit( $newTitle->getPrefixedText() ), array( 'type' => 'text', 'id' => 'wpNewTitle' ) ) .
 233+ Xml::input( 'wpNewTitle', 40, $wgContLang->recodeForEdit( $newTitle->getPrefixedText() ), array( 'type' => 'text', 'id' => 'wpNewTitle', 'maxlength' => 255 ) ) .
 234+ // maxLength enforced in JS.
234235 Html::hidden( 'wpOldTitle', $this->oldTitle->getPrefixedText() ) .
235236 "</td>
236237 </tr>
Index: trunk/phase3/resources/mediawiki.special/mediawiki.special.movePage.js
——@@ -2,4 +2,5 @@
33
44 jQuery( function( $ ) {
55 $( '#wpReason' ).byteLimit();
 6+ $( '#wpNewTitle' ).byteLimit();
67 });

Follow-up revisions

RevisionCommit summaryAuthorDate
r90257revert r87808 - Its broken, and looks rather complex to fix, especially with ...bawolff03:56, 17 June 2011

Comments

#Comment by Platonides (talk | contribs)   20:22, 19 May 2011

The maximum title length is not 255 but

#Comment by Bawolff (talk | contribs)   21:22, 19 May 2011

Good catch, I didn't think of that.

Not sure how to enforce that with the maxlength limiting. Might need to just revert this.

#Comment by Platonides (talk | contribs)   21:29, 19 May 2011

You could iterate all namespaces to find the upper limit. But it kind of breaks as you can easily get a limit enforced in the UI and then a lower one by the server, although JavaScript could detect namespaces and add their length to the limit.

#Comment by Krinkle (talk | contribs)   14:55, 17 June 2011

The pagename length is not a static number (ie. not 255, per Platonides), but the "title" (as in page_title) length is set to 255.

So what we want is to get only the title portion. Although extracting the title part from a string is complex to do ad-hoc (localization, aliases, canonical namespaces, lower/uppercase, trimming of whitespace), it's rather easy with mw.Title:

var title = new mw.Title( inputValue ); text.getMain(); // returns db-like version of the title, without namespace, ie. Foo_bar.jpg)

Although in order to be able to actually filter the inputValue, byteLimit needs the ability to pass a callback/filter function.

It's a little overhead to do now, but I think in a few days/weeks when the convenience dependancies are solved, this would be easy.


Recorded in BugZilla:

Status & tagging log