From 7fdbe15fb68f83b3af8baa5779123550eae19441 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Wed, 2 Nov 2016 13:13:43 -0400 Subject: [PATCH] HTMLForm: Allow returning Message objects from HTMLFormField::validate() It mostly already worked. HTMLForm::trySubmit() needed a little adjustment to handle things properly. Change-Id: Ibb17bb61ac0b2d41953249980bc2f23b8a3ae5b6 --- includes/htmlform/HTMLForm.php | 21 ++++++++++++------- includes/htmlform/HTMLFormField.php | 4 ++-- .../fields/HTMLAutoCompleteSelectField.php | 2 +- includes/htmlform/fields/HTMLButtonField.php | 2 +- includes/htmlform/fields/HTMLCheckMatrix.php | 2 +- .../htmlform/fields/HTMLDateTimeField.php | 8 +++---- includes/htmlform/fields/HTMLFloatField.php | 6 +++--- .../htmlform/fields/HTMLFormFieldCloner.php | 2 +- includes/htmlform/fields/HTMLIntField.php | 2 +- .../htmlform/fields/HTMLMultiSelectField.php | 2 +- includes/htmlform/fields/HTMLRadioField.php | 4 ++-- .../htmlform/fields/HTMLRestrictionsField.php | 6 +++--- .../fields/HTMLSelectAndOtherField.php | 2 +- includes/htmlform/fields/HTMLSelectField.php | 2 +- .../htmlform/fields/HTMLTitleTextField.php | 8 +++---- .../htmlform/fields/HTMLUserTextField.php | 4 ++-- .../includes/htmlform/HTMLCheckMatrixTest.php | 2 +- 17 files changed, 41 insertions(+), 38 deletions(-) diff --git a/includes/htmlform/HTMLForm.php b/includes/htmlform/HTMLForm.php index cadb5023e2..71ccaa36e5 100644 --- a/includes/htmlform/HTMLForm.php +++ b/includes/htmlform/HTMLForm.php @@ -604,10 +604,14 @@ class HTMLForm extends ContextSource { */ public function trySubmit() { $valid = true; - $hoistedErrors = []; - $hoistedErrors[] = isset( $this->mValidationErrorMessage ) - ? $this->mValidationErrorMessage - : [ 'htmlform-invalid-input' ]; + $hoistedErrors = Status::newGood(); + if ( $this->mValidationErrorMessage ) { + foreach ( (array)$this->mValidationErrorMessage as $error ) { + call_user_func_array( [ $hoistedErrors, 'fatal' ], $error ); + } + } else { + $hoistedErrors->fatal( 'htmlform-invalid-input' ); + } $this->mWasSubmitted = true; @@ -634,15 +638,16 @@ class HTMLForm extends ContextSource { if ( $res !== true ) { $valid = false; if ( $res !== false && !$field->canDisplayErrors() ) { - $hoistedErrors[] = [ 'rawmessage', $res ]; + if ( is_string( $res ) ) { + $hoistedErrors->fatal( 'rawmessage', $res ); + } else { + $hoistedErrors->fatal( $res ); + } } } } if ( !$valid ) { - if ( count( $hoistedErrors ) === 1 ) { - $hoistedErrors = $hoistedErrors[0]; - } return $hoistedErrors; } diff --git a/includes/htmlform/HTMLFormField.php b/includes/htmlform/HTMLFormField.php index 8390a0bbbe..71aa275449 100644 --- a/includes/htmlform/HTMLFormField.php +++ b/includes/htmlform/HTMLFormField.php @@ -296,7 +296,7 @@ abstract class HTMLFormField { * @param string|array $value The value the field was submitted with * @param array $alldata The data collected from the form * - * @return bool|string True on success, or String error to display, or + * @return bool|string|Message True on success, or String/Message error to display, or * false to fail validation without displaying an error. */ public function validate( $value, $alldata ) { @@ -308,7 +308,7 @@ abstract class HTMLFormField { && $this->mParams['required'] !== false && $value === '' ) { - return $this->msg( 'htmlform-required' )->parse(); + return $this->msg( 'htmlform-required' ); } if ( isset( $this->mValidationCallback ) ) { diff --git a/includes/htmlform/fields/HTMLAutoCompleteSelectField.php b/includes/htmlform/fields/HTMLAutoCompleteSelectField.php index 0f86ee8bae..87009f85d7 100644 --- a/includes/htmlform/fields/HTMLAutoCompleteSelectField.php +++ b/includes/htmlform/fields/HTMLAutoCompleteSelectField.php @@ -94,7 +94,7 @@ class HTMLAutoCompleteSelectField extends HTMLTextField { } elseif ( in_array( strval( $value ), $this->autocomplete, true ) ) { return true; } elseif ( $this->mParams['require-match'] ) { - return $this->msg( 'htmlform-select-badoption' )->parse(); + return $this->msg( 'htmlform-select-badoption' ); } return true; diff --git a/includes/htmlform/fields/HTMLButtonField.php b/includes/htmlform/fields/HTMLButtonField.php index 64fe7eda9b..285490bc98 100644 --- a/includes/htmlform/fields/HTMLButtonField.php +++ b/includes/htmlform/fields/HTMLButtonField.php @@ -113,7 +113,7 @@ class HTMLButtonField extends HTMLFormField { * @param string $value * @param array $alldata * - * @return bool + * @return bool|string|Message */ public function validate( $value, $alldata ) { return true; diff --git a/includes/htmlform/fields/HTMLCheckMatrix.php b/includes/htmlform/fields/HTMLCheckMatrix.php index 890cd7c3cc..46172a581b 100644 --- a/includes/htmlform/fields/HTMLCheckMatrix.php +++ b/includes/htmlform/fields/HTMLCheckMatrix.php @@ -65,7 +65,7 @@ class HTMLCheckMatrix extends HTMLFormField implements HTMLNestedFilterable { if ( count( $validValues ) == count( $value ) ) { return true; } else { - return $this->msg( 'htmlform-select-badoption' )->parse(); + return $this->msg( 'htmlform-select-badoption' ); } } diff --git a/includes/htmlform/fields/HTMLDateTimeField.php b/includes/htmlform/fields/HTMLDateTimeField.php index 88dcd2457b..5d4a15e856 100644 --- a/includes/htmlform/fields/HTMLDateTimeField.php +++ b/includes/htmlform/fields/HTMLDateTimeField.php @@ -100,15 +100,14 @@ class HTMLDateTimeField extends HTMLTextField { $date = $this->parseDate( $value ); if ( !$date ) { // Messages: htmlform-date-invalid htmlform-time-invalid htmlform-datetime-invalid - return $this->msg( "htmlform-{$this->mType}-invalid" )->parseAsBlock(); + return $this->msg( "htmlform-{$this->mType}-invalid" ); } if ( isset( $this->mParams['min'] ) ) { $min = $this->parseDate( $this->mParams['min'] ); if ( $min && $date < $min ) { // Messages: htmlform-date-toolow htmlform-time-toolow htmlform-datetime-toolow - return $this->msg( "htmlform-{$this->mType}-toolow", $this->formatDate( $min ) ) - ->parseAsBlock(); + return $this->msg( "htmlform-{$this->mType}-toolow", $this->formatDate( $min ) ); } } @@ -116,8 +115,7 @@ class HTMLDateTimeField extends HTMLTextField { $max = $this->parseDate( $this->mParams['max'] ); if ( $max && $date > $max ) { // Messages: htmlform-date-toohigh htmlform-time-toohigh htmlform-datetime-toohigh - return $this->msg( "htmlform-{$this->mType}-toohigh", $this->formatDate( $max ) ) - ->parseAsBlock(); + return $this->msg( "htmlform-{$this->mType}-toohigh", $this->formatDate( $max ) ); } } diff --git a/includes/htmlform/fields/HTMLFloatField.php b/includes/htmlform/fields/HTMLFloatField.php index d1250c06f7..d2d54e28d0 100644 --- a/includes/htmlform/fields/HTMLFloatField.php +++ b/includes/htmlform/fields/HTMLFloatField.php @@ -20,7 +20,7 @@ class HTMLFloatField extends HTMLTextField { # https://www.w3.org/TR/html5/infrastructure.html#floating-point-numbers # with the addition that a leading '+' sign is ok. if ( !preg_match( '/^((\+|\-)?\d+(\.\d+)?(E(\+|\-)?\d+)?)?$/i', $value ) ) { - return $this->msg( 'htmlform-float-invalid' )->parseAsBlock(); + return $this->msg( 'htmlform-float-invalid' ); } # The "int" part of these message names is rather confusing. @@ -29,7 +29,7 @@ class HTMLFloatField extends HTMLTextField { $min = $this->mParams['min']; if ( $min > $value ) { - return $this->msg( 'htmlform-int-toolow', $min )->parseAsBlock(); + return $this->msg( 'htmlform-int-toolow', $min ); } } @@ -37,7 +37,7 @@ class HTMLFloatField extends HTMLTextField { $max = $this->mParams['max']; if ( $max < $value ) { - return $this->msg( 'htmlform-int-toohigh', $max )->parseAsBlock(); + return $this->msg( 'htmlform-int-toohigh', $max ); } } diff --git a/includes/htmlform/fields/HTMLFormFieldCloner.php b/includes/htmlform/fields/HTMLFormFieldCloner.php index 5d8f491881..2584b6b874 100644 --- a/includes/htmlform/fields/HTMLFormFieldCloner.php +++ b/includes/htmlform/fields/HTMLFormFieldCloner.php @@ -224,7 +224,7 @@ class HTMLFormFieldCloner extends HTMLFormField { && $this->mParams['required'] !== false && !$values ) { - return $this->msg( 'htmlform-cloner-required' )->parseAsBlock(); + return $this->msg( 'htmlform-cloner-required' ); } if ( isset( $values['nonjs'] ) ) { diff --git a/includes/htmlform/fields/HTMLIntField.php b/includes/htmlform/fields/HTMLIntField.php index c87a778a4a..02af7de98b 100644 --- a/includes/htmlform/fields/HTMLIntField.php +++ b/includes/htmlform/fields/HTMLIntField.php @@ -18,7 +18,7 @@ class HTMLIntField extends HTMLFloatField { # input does not require its value to be numeric). If you want a tidier # value to, eg, save in the DB, clean it up with intval(). if ( !preg_match( '/^((\+|\-)?\d+)?$/', trim( $value ) ) ) { - return $this->msg( 'htmlform-int-invalid' )->parseAsBlock(); + return $this->msg( 'htmlform-int-invalid' ); } return true; diff --git a/includes/htmlform/fields/HTMLMultiSelectField.php b/includes/htmlform/fields/HTMLMultiSelectField.php index 58de763946..05a2ba6d5e 100644 --- a/includes/htmlform/fields/HTMLMultiSelectField.php +++ b/includes/htmlform/fields/HTMLMultiSelectField.php @@ -46,7 +46,7 @@ class HTMLMultiSelectField extends HTMLFormField implements HTMLNestedFilterable if ( count( $validValues ) == count( $value ) ) { return true; } else { - return $this->msg( 'htmlform-select-badoption' )->parse(); + return $this->msg( 'htmlform-select-badoption' ); } } diff --git a/includes/htmlform/fields/HTMLRadioField.php b/includes/htmlform/fields/HTMLRadioField.php index 69dc617be0..06ec3722ed 100644 --- a/includes/htmlform/fields/HTMLRadioField.php +++ b/includes/htmlform/fields/HTMLRadioField.php @@ -27,7 +27,7 @@ class HTMLRadioField extends HTMLFormField { } if ( !is_string( $value ) && !is_int( $value ) ) { - return $this->msg( 'htmlform-required' )->parse(); + return $this->msg( 'htmlform-required' ); } $validOptions = HTMLFormField::flattenOptions( $this->getOptions() ); @@ -35,7 +35,7 @@ class HTMLRadioField extends HTMLFormField { if ( in_array( strval( $value ), $validOptions, true ) ) { return true; } else { - return $this->msg( 'htmlform-select-badoption' )->parse(); + return $this->msg( 'htmlform-select-badoption' ); } } diff --git a/includes/htmlform/fields/HTMLRestrictionsField.php b/includes/htmlform/fields/HTMLRestrictionsField.php index 5a1802500b..dbf2c8f644 100644 --- a/includes/htmlform/fields/HTMLRestrictionsField.php +++ b/includes/htmlform/fields/HTMLRestrictionsField.php @@ -61,7 +61,7 @@ class HTMLRestrictionsField extends HTMLTextAreaField { * @param string|MWRestrictions $value The value the field was submitted with * @param array $alldata The data collected from the form * - * @return bool|string True on success, or String error to display, or + * @return bool|string|Message True on success, or String/Message error to display, or * false to fail validation without displaying an error. */ public function validate( $value, $alldata ) { @@ -73,7 +73,7 @@ class HTMLRestrictionsField extends HTMLTextAreaField { isset( $this->mParams['required'] ) && $this->mParams['required'] !== false && $value instanceof MWRestrictions && !$value->toArray()['IPAddresses'] ) { - return $this->msg( 'htmlform-required' )->parse(); + return $this->msg( 'htmlform-required' ); } if ( is_string( $value ) ) { @@ -87,7 +87,7 @@ class HTMLRestrictionsField extends HTMLTextAreaField { if ( $status->isOK() ) { $status->fatal( 'unknown-error' ); } - return $status->getMessage()->parse(); + return $status->getMessage(); } if ( isset( $this->mValidationCallback ) ) { diff --git a/includes/htmlform/fields/HTMLSelectAndOtherField.php b/includes/htmlform/fields/HTMLSelectAndOtherField.php index 86e8e75655..9af60e5c22 100644 --- a/includes/htmlform/fields/HTMLSelectAndOtherField.php +++ b/includes/htmlform/fields/HTMLSelectAndOtherField.php @@ -125,7 +125,7 @@ class HTMLSelectAndOtherField extends HTMLSelectField { && $this->mParams['required'] !== false && $value[1] === '' ) { - return $this->msg( 'htmlform-required' )->parse(); + return $this->msg( 'htmlform-required' ); } return true; diff --git a/includes/htmlform/fields/HTMLSelectField.php b/includes/htmlform/fields/HTMLSelectField.php index c1f8e4212d..18c741b77e 100644 --- a/includes/htmlform/fields/HTMLSelectField.php +++ b/includes/htmlform/fields/HTMLSelectField.php @@ -16,7 +16,7 @@ class HTMLSelectField extends HTMLFormField { if ( in_array( strval( $value ), $validOptions, true ) ) { return true; } else { - return $this->msg( 'htmlform-select-badoption' )->parse(); + return $this->msg( 'htmlform-select-badoption' ); } } diff --git a/includes/htmlform/fields/HTMLTitleTextField.php b/includes/htmlform/fields/HTMLTitleTextField.php index a15b90e333..3eb3f5dfa1 100644 --- a/includes/htmlform/fields/HTMLTitleTextField.php +++ b/includes/htmlform/fields/HTMLTitleTextField.php @@ -51,22 +51,22 @@ class HTMLTitleTextField extends HTMLTextField { if ( $params ) { $msg->params( $params ); } - return $msg->parse(); + return $msg; } $text = $title->getPrefixedText(); if ( $this->mParams['namespace'] !== false && !$title->inNamespace( $this->mParams['namespace'] ) ) { - return $this->msg( 'htmlform-title-badnamespace', $this->mParams['namespace'], $text )->parse(); + return $this->msg( 'htmlform-title-badnamespace', $this->mParams['namespace'], $text ); } if ( $this->mParams['creatable'] && !$title->canExist() ) { - return $this->msg( 'htmlform-title-not-creatable', $text )->escaped(); + return $this->msg( 'htmlform-title-not-creatable', $text ); } if ( $this->mParams['exists'] && !$title->exists() ) { - return $this->msg( 'htmlform-title-not-exists', $text )->parse(); + return $this->msg( 'htmlform-title-not-exists', $text ); } return parent::validate( $value, $alldata ); diff --git a/includes/htmlform/fields/HTMLUserTextField.php b/includes/htmlform/fields/HTMLUserTextField.php index 14b5e59573..12c09c1d7e 100644 --- a/includes/htmlform/fields/HTMLUserTextField.php +++ b/includes/htmlform/fields/HTMLUserTextField.php @@ -28,12 +28,12 @@ class HTMLUserTextField extends HTMLTextField { $user = User::newFromName( $value, false ); if ( !$user ) { - return $this->msg( 'htmlform-user-not-valid', $value )->parse(); + return $this->msg( 'htmlform-user-not-valid', $value ); } elseif ( ( $this->mParams['exists'] && $user->getId() === 0 ) && !( $this->mParams['ipallowed'] && User::isIP( $value ) ) ) { - return $this->msg( 'htmlform-user-not-exists', $user->getName() )->parse(); + return $this->msg( 'htmlform-user-not-exists', $user->getName() ); } return parent::validate( $value, $alldata ); diff --git a/tests/phpunit/includes/htmlform/HTMLCheckMatrixTest.php b/tests/phpunit/includes/htmlform/HTMLCheckMatrixTest.php index dd5b58bd5e..f97716b9e5 100644 --- a/tests/phpunit/includes/htmlform/HTMLCheckMatrixTest.php +++ b/tests/phpunit/includes/htmlform/HTMLCheckMatrixTest.php @@ -51,7 +51,7 @@ class HtmlCheckMatrixTest extends MediaWikiTestCase { public function testValidateAllowsOnlyKnownTags() { $field = new HTMLCheckMatrix( self::$defaultOptions ); - $this->assertInternalType( 'string', $this->validate( $field, [ 'foo' ] ) ); + $this->assertInstanceOf( Message::class, $this->validate( $field, [ 'foo' ] ) ); } public function testValidateAcceptsPartialTagList() { -- 2.20.1