diff --git a/includes/Status.php b/includes/Status.php index e5788733cf9..a6348b7165e 100644 --- a/includes/Status.php +++ b/includes/Status.php @@ -105,6 +105,26 @@ class Status { return new self( $sv ); } + /** + * Splits this Status object into two new Status objects, one which contains only + * the error messages, and one that contains the warnings, only. The returned array is + * defined as: + * array( + * 0 => object(Status) # the Status with error messages, only + * 1 => object(Status) # The Status with warning messages, only + * ) + * + * @return array + */ + public function splitByErrorType() { + list( $errorsOnlyStatusValue, $warningsOnlyStatusValue ) = $this->sv->splitByErrorType(); + $errorsOnlyStatus = new Status( $errorsOnlyStatusValue ); + $warningsOnlyStatus = new Status( $warningsOnlyStatusValue ); + $errorsOnlyStatus->cleanCallback = $warningsOnlyStatus->cleanCallback = $this->cleanCallback; + + return [ $errorsOnlyStatus, $warningsOnlyStatus ]; + } + /** * Change operation result * @@ -314,6 +334,7 @@ class Status { /** * Return the message for a single error. + * * @param mixed $error With an array & two values keyed by * 'message' and 'params', use those keys-value pairs. * Otherwise, if its an array, just use the first value as the diff --git a/includes/auth/AuthenticationResponse.php b/includes/auth/AuthenticationResponse.php index 0339e451b3c..6684fb958fc 100644 --- a/includes/auth/AuthenticationResponse.php +++ b/includes/auth/AuthenticationResponse.php @@ -81,6 +81,9 @@ class AuthenticationResponse { /** @var Message|null I18n message to display in case of UI or FAIL */ public $message = null; + /** @var string Whether the $message is an error or warning message, for styling reasons */ + public $messageType = 'warning'; + /** * @var string|null Local user name from authentication. * May be null if the authentication passed but no local user is known. @@ -144,6 +147,7 @@ class AuthenticationResponse { $ret = new AuthenticationResponse; $ret->status = AuthenticationResponse::FAIL; $ret->message = $msg; + $ret->messageType = 'error'; return $ret; } @@ -172,18 +176,23 @@ class AuthenticationResponse { /** * @param AuthenticationRequest[] $reqs AuthenticationRequests needed to continue * @param Message $msg + * @param string $msgtype * @return AuthenticationResponse * @see AuthenticationResponse::UI */ - public static function newUI( array $reqs, Message $msg ) { + public static function newUI( array $reqs, Message $msg, $msgtype = 'warning' ) { if ( !$reqs ) { throw new \InvalidArgumentException( '$reqs may not be empty' ); } + if ( $msgtype !== 'warning' && $msgtype !== 'error' ) { + throw new \InvalidArgumentException( $msgtype . ' is not a valid message type.' ); + } $ret = new AuthenticationResponse; $ret->status = AuthenticationResponse::UI; $ret->neededRequests = $reqs; $ret->message = $msg; + $ret->messageType = $msgtype; return $ret; } diff --git a/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php b/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php index beb11f43318..7f121cdef1d 100644 --- a/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php +++ b/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php @@ -64,7 +64,8 @@ class ConfirmLinkSecondaryAuthenticationProvider extends AbstractSecondaryAuthen $req = new ConfirmLinkAuthenticationRequest( $maybeLink ); return AuthenticationResponse::newUI( [ $req ], - wfMessage( 'authprovider-confirmlink-message' ) + wfMessage( 'authprovider-confirmlink-message' ), + 'warning' ); } @@ -150,7 +151,8 @@ class ConfirmLinkSecondaryAuthenticationProvider extends AbstractSecondaryAuthen 'linkOk', wfMessage( 'ok' ), wfMessage( 'authprovider-confirmlink-ok-help' ) ) ], - $combinedStatus->getMessage( 'authprovider-confirmlink-failed' ) + $combinedStatus->getMessage( 'authprovider-confirmlink-failed' ), + 'error' ); } } diff --git a/includes/auth/ResetPasswordSecondaryAuthenticationProvider.php b/includes/auth/ResetPasswordSecondaryAuthenticationProvider.php index dd97830dae9..f11a12c4214 100644 --- a/includes/auth/ResetPasswordSecondaryAuthenticationProvider.php +++ b/includes/auth/ResetPasswordSecondaryAuthenticationProvider.php @@ -112,17 +112,17 @@ class ResetPasswordSecondaryAuthenticationProvider extends AbstractSecondaryAuth $req = AuthenticationRequest::getRequestByClass( $reqs, get_class( $needReq ) ); if ( !$req || !array_key_exists( 'retype', $req->getFieldInfo() ) ) { - return AuthenticationResponse::newUI( $needReqs, $data->msg ); + return AuthenticationResponse::newUI( $needReqs, $data->msg, 'warning' ); } if ( $req->password !== $req->retype ) { - return AuthenticationResponse::newUI( $needReqs, new \Message( 'badretype' ) ); + return AuthenticationResponse::newUI( $needReqs, new \Message( 'badretype' ), 'error' ); } $req->username = $user->getName(); $status = $this->manager->allowsAuthenticationDataChange( $req ); if ( !$status->isGood() ) { - return AuthenticationResponse::newUI( $needReqs, $status->getMessage() ); + return AuthenticationResponse::newUI( $needReqs, $status->getMessage(), 'error' ); } $this->manager->changeAuthenticationData( $req ); diff --git a/includes/htmlform/HTMLForm.php b/includes/htmlform/HTMLForm.php index 3c88594eac7..567e692eddd 100644 --- a/includes/htmlform/HTMLForm.php +++ b/includes/htmlform/HTMLForm.php @@ -1014,7 +1014,8 @@ class HTMLForm extends ContextSource { $this->getOutput()->addModuleStyles( 'mediawiki.htmlform.styles' ); $html = '' - . $this->getErrors( $submitResult ) + . $this->getErrorsOrWarnings( $submitResult, 'error' ) + . $this->getErrorsOrWarnings( $submitResult, 'warning' ) . $this->getHeaderText() . $this->getBody() . $this->getHiddenFields() @@ -1230,23 +1231,46 @@ class HTMLForm extends ContextSource { * * @param string|array|Status $errors * + * @deprecated since 1.28, use getErrorsOrWarnings() instead + * * @return string */ public function getErrors( $errors ) { - if ( $errors instanceof Status ) { - if ( $errors->isOK() ) { - $errorstr = ''; + wfDeprecated( __METHOD__ ); + return $this->getErrorsOrWarnings( $errors, 'error' ); + } + + /** + * Returns a formatted list of errors or warnings from the given elements. + * + * @param string|array|Status $elements The set of errors/warnings to process. + * @param string $elementsType Should warnings or errors be returned. This is meant + * for Status objects, all other valid types are always considered as errors. + * @return string + */ + public function getErrorsOrWarnings( $elements, $elementsType ) { + if ( !in_array( $elementsType, [ 'error', 'warning' ], true ) ) { + throw new DomainException( $elementsType . ' is not a valid type.' ); + } + $elementstr = false; + if ( $elements instanceof Status ) { + list( $errorStatus, $warningStatus ) = $elements->splitByErrorType(); + $status = $elementsType === 'error' ? $errorStatus : $warningStatus; + if ( $status->isGood() ) { + $elementstr = ''; } else { - $errorstr = $this->getOutput()->parse( $errors->getWikiText() ); + $elementstr = $this->getOutput()->parse( + $status->getWikiText() + ); } - } elseif ( is_array( $errors ) ) { - $errorstr = $this->formatErrors( $errors ); - } else { - $errorstr = $errors; + } elseif ( is_array( $elements ) && $elementsType === 'error' ) { + $elementstr = $this->formatErrors( $elements ); + } elseif ( $elementsType === 'error' ) { + $elementstr = $elements; } - return $errorstr - ? Html::rawElement( 'div', [ 'class' => 'error' ], $errorstr ) + return $elementstr + ? Html::rawElement( 'div', [ 'class' => $elementsType ], $elementstr ) : ''; } diff --git a/includes/htmlform/OOUIHTMLForm.php b/includes/htmlform/OOUIHTMLForm.php index 0b227278a32..bbd3473af9f 100644 --- a/includes/htmlform/OOUIHTMLForm.php +++ b/includes/htmlform/OOUIHTMLForm.php @@ -26,6 +26,7 @@ */ class OOUIHTMLForm extends HTMLForm { private $oouiErrors; + private $oouiWarnings; public function __construct( $descriptor, $context = null, $messagePrefix = '' ) { parent::__construct( $descriptor, $context, $messagePrefix ); @@ -185,28 +186,34 @@ class OOUIHTMLForm extends HTMLForm { } /** - * @param string|array|Status $err + * @param string|array|Status $elements + * @param string $elementsType * @return string */ - function getErrors( $err ) { - if ( !$err ) { + function getErrorsOrWarnings( $elements, $elementsType ) { + if ( !in_array( $elementsType, [ 'error', 'warning' ] ) ) { + throw new DomainException( $elementsType . ' is not a valid type.' ); + } + if ( !$elements ) { $errors = []; - } elseif ( $err instanceof Status ) { - if ( $err->isOK() ) { + } elseif ( $elements instanceof Status ) { + if ( $elements->isGood() ) { $errors = []; } else { - $errors = $err->getErrorsByType( 'error' ); + $errors = $elements->getErrorsByType( $elementsType ); foreach ( $errors as &$error ) { // Input: array( 'message' => 'foo', 'errors' => array( 'a', 'b', 'c' ) ) // Output: array( 'foo', 'a', 'b', 'c' ) $error = array_merge( [ $error['message'] ], $error['params'] ); } } - } else { - $errors = $err; + } elseif ( $elementsType === 'errors' ) { + $errors = $elements; if ( !is_array( $errors ) ) { $errors = [ $errors ]; } + } else { + $errors = []; } foreach ( $errors as &$error ) { @@ -215,7 +222,11 @@ class OOUIHTMLForm extends HTMLForm { } // Used in getBody() - $this->oouiErrors = $errors; + if ( $elementsType === 'error' ) { + $this->oouiErrors = $errors; + } else { + $this->oouiWarnings = $errors; + } return ''; } @@ -236,7 +247,10 @@ class OOUIHTMLForm extends HTMLForm { if ( $this->oouiErrors ) { $classes[] = 'mw-htmlform-ooui-header-errors'; } - if ( $this->mHeader || $this->oouiErrors ) { + if ( $this->oouiWarnings ) { + $classes[] = 'mw-htmlform-ooui-header-warnings'; + } + if ( $this->mHeader || $this->oouiErrors || $this->oouiWarnings ) { // if there's no header, don't create an (empty) LabelWidget, simply use a placeholder if ( $this->mHeader ) { $element = new OOUI\LabelWidget( [ 'label' => new OOUI\HtmlSnippet( $this->mHeader ) ] ); @@ -249,6 +263,7 @@ class OOUIHTMLForm extends HTMLForm { [ 'align' => 'top', 'errors' => $this->oouiErrors, + 'notices' => $this->oouiWarnings, 'classes' => $classes, ] ) diff --git a/includes/libs/StatusValue.php b/includes/libs/StatusValue.php index 1d23f9d368a..f45dc41248c 100644 --- a/includes/libs/StatusValue.php +++ b/includes/libs/StatusValue.php @@ -79,6 +79,34 @@ class StatusValue { return $result; } + /** + * Splits this StatusValue object into two new StatusValue objects, one which contains only + * the error messages, and one that contains the warnings, only. The returned array is + * defined as: + * array( + * 0 => object(StatusValue) # the StatusValue with error messages, only + * 1 => object(StatusValue) # The StatusValue with warning messages, only + * ) + * + * @return array + */ + public function splitByErrorType() { + $errorsOnlyStatusValue = clone $this; + $warningsOnlyStatusValue = clone $this; + $warningsOnlyStatusValue->ok = true; + + $errorsOnlyStatusValue->errors = $warningsOnlyStatusValue->errors = []; + foreach ( $this->errors as $item ) { + if ( $item['type'] === 'warning' ) { + $warningsOnlyStatusValue->errors[] = $item; + } else { + $errorsOnlyStatusValue->errors[] = $item; + } + }; + + return [ $errorsOnlyStatusValue, $warningsOnlyStatusValue ]; + } + /** * Returns whether the operation completed and didn't have any error or * warnings diff --git a/includes/specialpage/LoginSignupSpecialPage.php b/includes/specialpage/LoginSignupSpecialPage.php index c3d43df2664..9e9397017c8 100644 --- a/includes/specialpage/LoginSignupSpecialPage.php +++ b/includes/specialpage/LoginSignupSpecialPage.php @@ -354,7 +354,7 @@ abstract class LoginSignupSpecialPage extends AuthManagerSpecialPage { $this->authAction = $this->isSignup() ? AuthManager::ACTION_CREATE_CONTINUE : AuthManager::ACTION_LOGIN_CONTINUE; $this->authRequests = $response->neededRequests; - $this->mainLoginForm( $response->neededRequests, $response->message, 'warning' ); + $this->mainLoginForm( $response->neededRequests, $response->message, $response->messageType ); break; default: throw new LogicException( 'invalid AuthenticationResponse' ); @@ -494,7 +494,13 @@ abstract class LoginSignupSpecialPage extends AuthManagerSpecialPage { $form = $this->getAuthForm( $requests, $this->authAction, $msg, $msgtype ); $form->prepareForm(); - $formHtml = $form->getHTML( $msg ? Status::newFatal( $msg ) : false ); + $submitStatus = Status::newGood(); + if ( $msg && $msgtype === 'warning' ) { + $submitStatus->warning( $msg ); + } elseif ( $msg && $msgtype === 'error' ) { + $submitStatus->fatal( $msg ); + } + $formHtml = $form->getHTML( $submitStatus ); $out->addHTML( $this->getPageHtml( $formHtml ) ); } diff --git a/resources/src/mediawiki.less/mediawiki.ui/variables.less b/resources/src/mediawiki.less/mediawiki.ui/variables.less index 507109ae2c0..b6f656878c1 100644 --- a/resources/src/mediawiki.less/mediawiki.ui/variables.less +++ b/resources/src/mediawiki.less/mediawiki.ui/variables.less @@ -51,6 +51,7 @@ @colorButtonTextActive: @colorGray7; @colorDisabledText: @colorGray12; @colorErrorText: #c00; +@colorWarningText: #705000; // UI colors @colorFieldBorder: @colorGray12; diff --git a/resources/src/mediawiki.ui/components/forms.less b/resources/src/mediawiki.ui/components/forms.less index cc96a5c09c1..aedec5b9e96 100644 --- a/resources/src/mediawiki.ui/components/forms.less +++ b/resources/src/mediawiki.ui/components/forms.less @@ -103,6 +103,7 @@ // // Styleguide 5.2. .error, + .warning, .errorbox, .warningbox, .successbox { @@ -118,7 +119,13 @@ color: @colorErrorText; border: 1px solid #fac5c5; background-color: #fae3e3; - text-shadow: 0 1px #fae3e3; + } + + // Colours taken from those for .warningbox in shared.css + .warning { + color: @colorWarningText; + border: 1px solid #fde29b; + background-color: #fdf1d1; } // This specifies styling for individual field validation error messages. diff --git a/tests/phpunit/includes/StatusTest.php b/tests/phpunit/includes/StatusTest.php index 474a481a6e5..ebc2d1080c6 100644 --- a/tests/phpunit/includes/StatusTest.php +++ b/tests/phpunit/includes/StatusTest.php @@ -645,4 +645,66 @@ class StatusTest extends MediaWikiLangTestCase { ]; } + /** + * @dataProvider provideErrorsWarningsOnly + * @covers Status::getErrorsOnlyStatus + * @covers Status::getWarningsOnlyStatus + */ + public function testGetErrorsWarningsOnlyStatus( $errorText, $warningText, $type, $errorResult, + $warningResult + ) { + $status = Status::newGood(); + if ( $errorText ) { + $status->fatal( $errorText ); + } + if ( $warningText ) { + $status->warning( $warningText ); + } + $testStatus = $status->splitByErrorType()[$type]; + $this->assertEquals( $errorResult, $testStatus->getErrorsByType( 'error' ) ); + $this->assertEquals( $warningResult, $testStatus->getErrorsByType( 'warning' ) ); + } + + public static function provideErrorsWarningsOnly() { + return [ + [ + 'Just an error', + 'Just a warning', + 0, + [ + 0 => [ + 'type' => 'error', + 'message' => 'Just an error', + 'params' => [] + ], + ], + [], + ], [ + 'Just an error', + 'Just a warning', + 1, + [], + [ + 0 => [ + 'type' => 'warning', + 'message' => 'Just a warning', + 'params' => [] + ], + ], + ], [ + null, + null, + 1, + [], + [], + ], [ + null, + null, + 0, + [], + [], + ] + ]; + } + } diff --git a/tests/phpunit/includes/auth/AuthenticationResponseTest.php b/tests/phpunit/includes/auth/AuthenticationResponseTest.php index 477b1617f5a..194b49e01ac 100644 --- a/tests/phpunit/includes/auth/AuthenticationResponseTest.php +++ b/tests/phpunit/includes/auth/AuthenticationResponseTest.php @@ -16,6 +16,7 @@ class AuthenticationResponseTest extends \MediaWikiTestCase { public function testConstructors( $constructor, $args, $expect ) { if ( is_array( $expect ) ) { $res = new AuthenticationResponse(); + $res->messageType = 'warning'; foreach ( $expect as $field => $value ) { $res->$field = $value; } @@ -51,6 +52,7 @@ class AuthenticationResponseTest extends \MediaWikiTestCase { [ 'newFail', [ $msg ], [ 'status' => AuthenticationResponse::FAIL, 'message' => $msg, + 'messageType' => 'error', ] ], [ 'newRestart', [ $msg ], [ @@ -66,6 +68,21 @@ class AuthenticationResponseTest extends \MediaWikiTestCase { 'status' => AuthenticationResponse::UI, 'neededRequests' => [ $req ], 'message' => $msg, + 'messageType' => 'warning', + ] ], + + [ 'newUI', [ [ $req ], $msg, 'warning' ], [ + 'status' => AuthenticationResponse::UI, + 'neededRequests' => [ $req ], + 'message' => $msg, + 'messageType' => 'warning', + ] ], + + [ 'newUI', [ [ $req ], $msg, 'error' ], [ + 'status' => AuthenticationResponse::UI, + 'neededRequests' => [ $req ], + 'message' => $msg, + 'messageType' => 'error', ] ], [ 'newUI', [ [], $msg ], new \InvalidArgumentException( '$reqs may not be empty' )