Change Special:ChangePassword to use OOUI
authorFlorian <florian.schmidt.stargatewissen@gmail.com>
Tue, 15 Sep 2015 17:54:02 +0000 (19:54 +0200)
committerFlorian <florian.schmidt.stargatewissen@gmail.com>
Tue, 3 Nov 2015 15:06:22 +0000 (16:06 +0100)
In order to work with OOUI properly, SpecialChangePassword::attemptReset() doesn't
throw PasswordErrors anymore. It now returns a Status object which
is set to fatal, if some requirement doesn't meet.

In OOUIHTMLForm: Handle Message objects as errors correctly. It's a valid parameter
for fatal to use a Message object instead of a message key, so OOUIHTMLForm needs
to handle Mesage objects as Message objects (can be parsed directly) instead of using
it as a message key (which usually doesn't exist).

Bug: T78373
Change-Id: Id768833bbd966cdeadd5a13fdb64b636ea2062ef

includes/htmlform/OOUIHTMLForm.php
includes/specials/SpecialChangePassword.php

index 60f02a1..d328ecc 100644 (file)
@@ -160,7 +160,12 @@ class OOUIHTMLForm extends HTMLForm {
                                $msg = $error;
                                $error = array();
                        }
-                       $error = $this->msg( $msg, $error )->parse();
+                       // if the error is already a message object, don't use it as a message key
+                       if ( !$msg instanceof Message ) {
+                               $error = $this->msg( $msg, $error )->parse();
+                       } else {
+                               $error = $msg->parse();
+                       }
                        $error = new OOUI\HtmlSnippet( $error );
                }
 
index df68b12..91ac4e0 100644 (file)
@@ -190,20 +190,16 @@ class SpecialChangePassword extends FormSpecialPage {
                        return true;
                }
 
-               try {
-                       $this->mUserName = $request->getVal( 'wpName', $this->getUser()->getName() );
-                       $this->mDomain = $wgAuth->getDomain();
+               $this->mUserName = $request->getVal( 'wpName', $this->getUser()->getName() );
+               $this->mDomain = $wgAuth->getDomain();
 
-                       if ( !$wgAuth->allowPasswordChange() ) {
-                               throw new ErrorPageError( 'changepassword', 'resetpass_forbidden' );
-                       }
+               if ( !$wgAuth->allowPasswordChange() ) {
+                       throw new ErrorPageError( 'changepassword', 'resetpass_forbidden' );
+               }
 
-                       $this->attemptReset( $data['Password'], $data['NewPassword'], $data['Retype'] );
+               $status = $this->attemptReset( $data['Password'], $data['NewPassword'], $data['Retype'] );
 
-                       return true;
-               } catch ( PasswordError $e ) {
-                       return $e->getMessage();
-               }
+               return $status;
        }
 
        public function onSuccess() {
@@ -231,10 +227,14 @@ class SpecialChangePassword extends FormSpecialPage {
        }
 
        /**
-        * @param string $oldpass
-        * @param string $newpass
-        * @param string $retype
-        * @throws PasswordError When cannot set the new password because requirements not met.
+        * Checks the new password if it meets the requirements for passwords and set
+        * it as a current password, otherwise set the passed Status object to fatal
+        * and doesn't change anything
+        *
+        * @param string $oldpass The current (temporary) password.
+        * @param string $newpass The password to set.
+        * @param string $retype The string of the retype password field to check with newpass
+        * @return Status
         */
        protected function attemptReset( $oldpass, $newpass, $retype ) {
                $isSelf = ( $this->mUserName === $this->getUser()->getName() );
@@ -245,33 +245,32 @@ class SpecialChangePassword extends FormSpecialPage {
                }
 
                if ( !$user || $user->isAnon() ) {
-                       throw new PasswordError( $this->msg( 'nosuchusershort', $this->mUserName )->text() );
+                       return Status::newFatal( $this->msg( 'nosuchusershort', $this->mUserName ) );
                }
 
                if ( $newpass !== $retype ) {
                        Hooks::run( 'PrefsPasswordAudit', array( $user, $newpass, 'badretype' ) );
-                       throw new PasswordError( $this->msg( 'badretype' )->text() );
+                       return Status::newFatal( $this->msg( 'badretype' ) );
                }
 
                $throttleCount = LoginForm::incLoginThrottle( $this->mUserName );
                if ( $throttleCount === true ) {
                        $lang = $this->getLanguage();
                        $throttleInfo = $this->getConfig()->get( 'PasswordAttemptThrottle' );
-                       throw new PasswordError( $this->msg( 'changepassword-throttled' )
+                       return Status::newFatal( $this->msg( 'changepassword-throttled' )
                                ->params( $lang->formatDuration( $throttleInfo['seconds'] ) )
-                               ->text()
                        );
                }
 
                // @todo Make these separate messages, since the message is written for both cases
                if ( !$user->checkTemporaryPassword( $oldpass ) && !$user->checkPassword( $oldpass ) ) {
                        Hooks::run( 'PrefsPasswordAudit', array( $user, $newpass, 'wrongpassword' ) );
-                       throw new PasswordError( $this->msg( 'resetpass-wrong-oldpass' )->text() );
+                       return Status::newFatal( $this->msg( 'resetpass-wrong-oldpass' ) );
                }
 
                // User is resetting their password to their old password
                if ( $oldpass === $newpass ) {
-                       throw new PasswordError( $this->msg( 'resetpass-recycled' )->text() );
+                       return Status::newFatal( $this->msg( 'resetpass-recycled' ) );
                }
 
                // Do AbortChangePassword after checking mOldpass, so we don't leak information
@@ -279,7 +278,7 @@ class SpecialChangePassword extends FormSpecialPage {
                $abortMsg = 'resetpass-abort-generic';
                if ( !Hooks::run( 'AbortChangePassword', array( $user, $oldpass, $newpass, &$abortMsg ) ) ) {
                        Hooks::run( 'PrefsPasswordAudit', array( $user, $newpass, 'abortreset' ) );
-                       throw new PasswordError( $this->msg( $abortMsg )->text() );
+                       return Status::newFatal( $this->msg( $abortMsg ) );
                }
 
                // Please reset throttle for successful logins, thanks!
@@ -292,7 +291,7 @@ class SpecialChangePassword extends FormSpecialPage {
                        Hooks::run( 'PrefsPasswordAudit', array( $user, $newpass, 'success' ) );
                } catch ( PasswordError $e ) {
                        Hooks::run( 'PrefsPasswordAudit', array( $user, $newpass, 'error' ) );
-                       throw new PasswordError( $e->getMessage() );
+                       return Status::newFatal( new RawMessage( $e->getMessage() ) );
                }
 
                if ( $isSelf ) {
@@ -303,6 +302,7 @@ class SpecialChangePassword extends FormSpecialPage {
                }
                $user->saveSettings();
                $this->resetPasswordExpiration( $user );
+               return Status::newGood();
        }
 
        public function requiresUnblock() {
@@ -336,4 +336,8 @@ class SpecialChangePassword extends FormSpecialPage {
                        __METHOD__
                );
        }
+
+       protected function getDisplayFormat() {
+               return 'ooui';
+       }
 }