r56896 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r56895‎ | r56896 | r56897 >
Date:21:25, 24 September 2009
Author:happy-melon
Status:reverted
Tags:
Comment:
Improve HTMLForm's support for adding content in and around the form, to improve versatility. Groundwork for being able to validate, filter and submit the CreateAccount input via HTMLForm::trySubmit(), which will give extensions huge flexibility to play with the form input.

Also change the wrapping on both forms to use a <fieldset>, which is visually more consistent with the other forms we have floating around the site. Add visualClear to all HTMLForm forms, and remove all the redundant CSS for the login form, quite a bit of which has been dead for a while.
Modified paths:
  • /trunk/phase3/includes/HTMLForm.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialCreateAccount.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserlogin.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)
  • /trunk/phase3/skins/monobook/main.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/monobook/main.css
——@@ -1015,59 +1015,10 @@
10161016 margin-top: 2em;
10171017 }
10181018
1019 -div#userloginForm form,
1020 -div#userlogin form#userlogin2 {
1021 - margin: 0 3em 1em 0;
1022 - border: 1px solid #aaa;
1023 - clear: both;
1024 - padding: 1.5em 2em;
1025 - background-color: #f9f9f9;
1026 - float: left;
1027 -}
1028 -.rtl div#userloginForm form,
1029 -.rtl div#userlogin form#userlogin2 {
1030 - float: right;
1031 -}
1032 -
1033 -div#userloginForm table,
1034 -div#userlogin form#userlogin2 table {
1035 - background-color: #f9f9f9;
1036 -}
1037 -
1038 -div#userloginForm h2,
1039 -div#userlogin form#userlogin2 h2 {
1040 - padding-top: 0;
1041 -}
1042 -
1043 -div#userlogin .captcha,
1044 -div#userloginForm .captcha {
1045 - border: 1px solid #bbb;
1046 - padding: 1.5em 2em;
1047 - background-color: white;
1048 -}
1049 -
1050 -#loginend, #signupend {
1051 - clear: both;
1052 -}
1053 -
10541019 #userloginprompt, #languagelinks {
10551020 font-size: 85%;
10561021 }
10571022
1058 -#login-sectiontip {
1059 - font-size: 85%;
1060 - line-height: 1.2;
1061 - padding-top: 2em;
1062 -}
1063 -
1064 -#userlogin .loginText, #userlogin .loginPassword {
1065 - width: 12em;
1066 -}
1067 -
1068 -#userloginlink a, #wpLoginattempt, #wpCreateaccount {
1069 - font-weight: bold;
1070 -}
1071 -
10721023 /*
10731024 ** IE/Mac fixes, hope to find a validating way to move this
10741025 ** to a separate stylesheet. This would work but doesn't validate:
Index: trunk/phase3/includes/HTMLForm.php
——@@ -73,17 +73,24 @@
7474 protected $mMessagePrefix;
7575 protected $mFlatFields;
7676 protected $mFieldTree;
77 - protected $mShowReset;
 77+ protected $mShowReset = false;
7878 public $mFieldData;
 79+
7980 protected $mSubmitCallback;
8081 protected $mValidationErrorMessage;
81 - protected $mIntro;
 82+
 83+ protected $mPre = '';
 84+ protected $mHeader = '';
 85+ protected $mPost = '';
 86+
8287 protected $mSubmitID;
8388 protected $mSubmitText;
8489 protected $mTitle;
8590
8691 protected $mHiddenFields = array();
8792 protected $mButtons = array();
 93+
 94+ protected $mWrapperLegend = false;
8895
8996 /**
9097 * Build a new HTMLForm from an array of field attributes
——@@ -127,8 +134,6 @@
128135 }
129136
130137 $this->mFieldTree = $loadedDescriptor;
131 -
132 - $this->mShowReset = true;
133138 }
134139
135140 /**
——@@ -247,16 +252,32 @@
248253 function setValidationErrorMessage( $msg ) {
249254 $this->mValidationErrorMessage = $msg;
250255 }
 256+
 257+ /**
 258+ * Set the introductory message, overwriting any existing message.
 259+ * @param $msg String complete text of message to display
 260+ */
 261+ function setIntro( $msg ) { $this->mPre = $msg; }
251262
252263 /**
253 - * Set the introductory message
 264+ * Add introductory text.
254265 * @param $msg String complete text of message to display
255266 */
256 - function setIntro( $msg ) {
257 - $this->mIntro = $msg;
258 - }
 267+ function addPreText( $msg ) { $this->mPre .= $msg; }
259268
260269 /**
 270+ * Add header text, inside the form.
 271+ * @param $msg String complete text of message to display
 272+ */
 273+ function addHeaderText( $msg ) { $this->mHeader .= $msg; }
 274+
 275+ /**
 276+ * Add text to the end of the display.
 277+ * @param $msg String complete text of message to display
 278+ */
 279+ function addPostText( $msg ) { $this->mPost .= $msg; }
 280+
 281+ /**
261282 * Add a hidden field to the output
262283 * @param $name String field name
263284 * @param $value String field value
——@@ -281,21 +302,20 @@
282303 $this->displayErrors( $submitResult );
283304 }
284305
285 - if ( isset( $this->mIntro ) ) {
286 - $wgOut->addHTML( $this->mIntro );
287 - }
 306+ $html = ''
 307+ . $this->mHeader
 308+ . $this->getBody()
 309+ . $this->getHiddenFields()
 310+ . $this->getButtons()
 311+ ;
288312
289 - $html = $this->getBody();
290 -
291 - // Hidden fields
292 - $html .= $this->getHiddenFields();
293 -
294 - // Buttons
295 - $html .= $this->getButtons();
296 -
297313 $html = $this->wrapForm( $html );
298314
299 - $wgOut->addHTML( $html );
 315+ $wgOut->addHTML( ''
 316+ . $this->mPre
 317+ . $html
 318+ . $this->mPost
 319+ );
300320 }
301321
302322 /**
——@@ -304,11 +324,18 @@
305325 * @return String wrapped HTML.
306326 */
307327 function wrapForm( $html ) {
 328+
 329+ # Include a <fieldset> wrapper for style, if requested.
 330+ if( $this->mWrapperLegend !== false ){
 331+ $html = Xml::fieldset( $this->mWrapperLegend, $html );
 332+ }
 333+
308334 return Html::rawElement(
309335 'form',
310336 array(
311337 'action' => $this->getTitle()->getFullURL(),
312338 'method' => 'post',
 339+ 'class' => 'visualClear',
313340 ),
314341 $html
315342 );
——@@ -447,6 +474,14 @@
448475 function setSubmitID( $t ) {
449476 $this->mSubmitID = $t;
450477 }
 478+
 479+ /**
 480+ * Prompt the whole form to be wrapped in a <fieldset>, with
 481+ * this text as its <legend> element.
 482+ * @param $legend String HTML to go inside the <legend> element.
 483+ * Will be escaped
 484+ */
 485+ public function setWrapperLegend( $legend ){ $this->mWrapperLegend = $legend; }
451486
452487 /**
453488 * Set the prefix for various default messages
Index: trunk/phase3/includes/specials/SpecialUserlogin.php
——@@ -222,10 +222,35 @@
223223 $form->setSubmitText( wfMsg( 'login' ) );
224224 $form->setSubmitId( 'wpLoginAttempt' );
225225 $form->suppressReset();
226 - $form->loadData();
 226+ $form->setWrapperLegend( wfMsg( 'userlogin' ) );
 227+
227228 $form->addHiddenField( 'returnto', $this->mReturnTo );
228229 $form->addHiddenField( 'returntoquery', $this->mReturnToQuery );
229230
 231+ $form->addHeaderText( ''
 232+ . Html::rawElement( 'p', array( 'id' => 'userloginlink' ),
 233+ $link )
 234+ . Html::rawElement( 'div', array( 'id' => 'userloginprompt' ),
 235+ wfMsgExt( 'loginprompt', array( 'parseinline' ) ) )
 236+ . $this->mFormHeader
 237+ . $langSelector
 238+ );
 239+ $form->addPreText( ''
 240+ . $msg
 241+ . Html::rawElement(
 242+ 'div',
 243+ array( 'id' => 'loginstart' ),
 244+ wfMsgExt( 'loginstart', array( 'parseinline' ) )
 245+ )
 246+ );
 247+ $form->addPostText(
 248+ Html::rawElement(
 249+ 'div',
 250+ array( 'id' => 'loginend' ),
 251+ wfMsgExt( 'loginend', array( 'parseinline' ) )
 252+ )
 253+ );
 254+
230255 # Add a 'mail reset' button if available
231256 $buttons = '';
232257 if( $wgEnableEmail && $wgAuth->allowPasswordChange() ){
——@@ -236,41 +261,14 @@
237262 );
238263 }
239264
240 - $formContents = ''
241 - . Html::rawElement( 'p', array( 'id' => 'userloginlink' ),
242 - $link )
243 - . Html::rawElement( 'div', array( 'id' => 'userloginprompt' ),
244 - wfMsgExt( 'loginprompt', array( 'parseinline' ) ) )
245 - . $this->mFormHeader
246 - . $langSelector
247 - . $form->getBody()
248 - . $form->getHiddenFields()
249 - . $form->getButtons()
250 - ;
 265+ $form->loadData();
251266
252267 $wgOut->setPageTitle( wfMsg( 'login' ) );
253268 $wgOut->setRobotPolicy( 'noindex,nofollow' );
254269 $wgOut->setArticleRelated( false );
255270 $wgOut->disallowUserJs(); # Stop malicious userscripts sniffing passwords
256271
257 - $wgOut->addHTML(
258 - Html::rawElement(
259 - 'div',
260 - array( 'id' => 'loginstart' ),
261 - wfMsgExt( 'loginstart', array( 'parseinline' ) )
262 - ) .
263 - $msg .
264 - Html::rawElement(
265 - 'div',
266 - array( 'id' => 'userloginForm' ),
267 - $form->wrapForm( $formContents )
268 - ) .
269 - Html::rawElement(
270 - 'div',
271 - array( 'id' => 'loginend' ),
272 - wfMsgExt( 'loginend', array( 'parseinline' ) )
273 - )
274 - );
 272+ $form->displayForm( '' );
275273 }
276274
277275 /**
Index: trunk/phase3/includes/specials/SpecialCreateAccount.php
——@@ -377,10 +377,10 @@
378378 unset( $this->mFormFields['Email'] );
379379 } else {
380380 if( $wgEmailConfirmToEdit ){
381 - $this->mFormFields['Email']['label-help'] = 'prefs-help-email-required';
 381+ $this->mFormFields['Email']['help-message'] = 'prefs-help-email-required';
382382 $this->mFormFields['Email']['required'] = '';
383383 } else {
384 - $this->mFormFields['Email']['label-help'] = 'prefs-help-email';
 384+ $this->mFormFields['Email']['help-message'] = 'prefs-help-email';
385385 }
386386 }
387387
——@@ -399,15 +399,39 @@
400400 $this->mFormFields['Remember']['checked'] = '1';
401401 }
402402
403 - $form = new HTMLForm( $this->mFormFields, '' );
 403+ $form = new HTMLForm( $this->mFormFields );
 404+
404405 $form->setTitle( $this->getTitle() );
405406 $form->setSubmitText( wfMsg( 'createaccount' ) );
406407 $form->setSubmitId( 'wpCreateaccount' );
407408 $form->suppressReset();
408 - $form->loadData();
 409+ $form->setWrapperLegend( wfMsg( 'createaccount' ) );
 410+
409411 $form->addHiddenField( 'returnto', $this->mReturnTo );
410412 $form->addHiddenField( 'returntoquery', $this->mReturnToQuery );
411413
 414+ $form->addHeaderText( ''
 415+ . Html::rawElement( 'p', array( 'id' => 'userloginlink' ),
 416+ $link )
 417+ . $this->mFormHeader
 418+ . $langSelector
 419+ );
 420+ $form->addPreText( ''
 421+ . $msg
 422+ . Html::rawElement(
 423+ 'div',
 424+ array( 'id' => 'signupstart' ),
 425+ wfMsgExt( 'loginstart', array( 'parseinline' ) )
 426+ )
 427+ );
 428+ $form->addPostText(
 429+ Html::rawElement(
 430+ 'div',
 431+ array( 'id' => 'signupend' ),
 432+ wfMsgExt( 'loginend', array( 'parseinline' ) )
 433+ )
 434+ );
 435+
412436 # Add a 'send password by email' button if available
413437 if( $wgEnableEmail && $wgUser->isLoggedIn() ){
414438 $form->addButton(
——@@ -417,40 +441,16 @@
418442 );
419443 }
420444
421 - $formContents = ''
422 - . Html::rawElement( 'p', array( 'id' => 'userloginlink' ),
423 - $link )
424 - . $this->mFormHeader
425 - . $langSelector
426 - . $form->getBody()
427 - . $form->getHiddenFields()
428 - . $form->getButtons()
429 - ;
 445+ $form->loadData();
430446
431447 $wgOut->setPageTitle( wfMsg( 'createaccount' ) );
432448 $wgOut->setRobotPolicy( 'noindex,nofollow' );
433449 $wgOut->setArticleRelated( false );
434450 $wgOut->disallowUserJs(); # Stop malicious userscripts sniffing passwords
435451
436 - $wgOut->addHTML(
437 - Html::rawElement(
438 - 'div',
439 - array( 'id' => 'loginstart' ),
440 - wfMsgExt( 'loginstart', array( 'parseinline' ) )
441 - ) .
442 - $msg .
443 - Html::rawElement(
444 - 'div',
445 - array( 'id' => 'userloginForm' ),
446 - $form->wrapForm( $formContents )
447 - ) .
448 - Html::rawElement(
449 - 'div',
450 - array( 'id' => 'loginend' ),
451 - wfMsgExt( 'loginend', array( 'parseinline' ) )
452 - )
453 - );
454452
 453+ $form->displayForm( false );
 454+
455455 }
456456
457457 /**
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -1048,10 +1048,10 @@
10491049 'logout' => 'Log out',
10501050 'userlogout' => 'Log out',
10511051 'notloggedin' => 'Not logged in',
1052 -'nologin' => "Don't have an account? $1.",
 1052+'nologin' => "Don't have an account? '''$1'''.",
10531053 'nologinlink' => 'Create an account',
10541054 'createaccount' => 'Create account',
1055 -'gotaccount' => 'Already have an account? $1.',
 1055+'gotaccount' => "Already have an account? '''$1'''.",
10561056 'gotaccountlink' => 'Log in',
10571057 'createaccountmail' => 'by e-mail',
10581058 'badretype' => 'The passwords you entered do not match.',

Follow-up revisions

RevisionCommit summaryAuthorDate
r56937Revert broken rewrite of login system; totally broken....brion00:49, 26 September 2009
r57024Recommit some of the 'backend' stuff from the Login branch, after talking wit...happy-melon19:04, 28 September 2009

Status & tagging log