diff --git a/core/core.services.yml b/core/core.services.yml index 589c5f6c135f437ba5f55597a2b16a61b3ce7910..9f83cecd8023cd81903a44e2f8c573c05c4c1855 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -285,10 +285,13 @@ services: arguments: ['@form_validator', '@form_submitter', '@form_cache', '@module_handler', '@event_dispatcher', '@request_stack', '@class_resolver', '@element_info', '@theme.manager', '@?csrf_token'] form_validator: class: Drupal\Core\Form\FormValidator - arguments: ['@request_stack', '@string_translation', '@csrf_token', '@logger.channel.form'] + arguments: ['@request_stack', '@string_translation', '@csrf_token', '@logger.channel.form', '@form_error_handler'] form_submitter: class: Drupal\Core\Form\FormSubmitter arguments: ['@request_stack', '@url_generator'] + form_error_handler: + class: Drupal\Core\Form\FormErrorHandler + arguments: ['@string_translation', '@link_generator'] form_cache: class: Drupal\Core\Form\FormCache arguments: ['@app.root', '@keyvalue.expirable', '@module_handler', '@current_user', '@csrf_token', '@logger.channel.form', '@request_stack', '@page_cache_request_policy'] diff --git a/core/includes/form.inc b/core/includes/form.inc index 01d9068d9c215d70c33bce5d161b53e7077bc70e..817082010f634a41aee91daf2401763ec4f642f0 100644 --- a/core/includes/form.inc +++ b/core/includes/form.inc @@ -11,6 +11,7 @@ use Drupal\Component\Utility\Xss; use Drupal\Core\Database\Database; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Form\FormElementHelper; use Drupal\Core\Form\OptGroup; use Drupal\Core\Render\Element; use Drupal\Core\Template\Attribute; @@ -203,6 +204,12 @@ function template_preprocess_fieldset(&$variables) { // Add the description's id to the fieldset aria attributes. $variables['attributes']['aria-describedby'] = $description_id; } + + // Display any error messages. + $variables['errors'] = NULL; + if (!empty($element['#errors']) && empty($element['#error_no_message'])) { + $variables['errors'] = $element['#errors']; + } } /** @@ -407,7 +414,7 @@ function template_preprocess_form_element(&$variables) { ); $variables['attributes'] = $element['#wrapper_attributes']; - // Add element #id for #type 'item'. + // Add element #id for #type 'item' and 'password_confirm'. if (isset($element['#markup']) && !empty($element['#id'])) { $variables['attributes']['id'] = $element['#id']; } @@ -423,6 +430,12 @@ function template_preprocess_form_element(&$variables) { // Pass elements disabled status to template. $variables['disabled'] = !empty($element['#attributes']['disabled']) ? $element['#attributes']['disabled'] : NULL; + // Display any error messages. + $variables['errors'] = NULL; + if (!empty($element['#errors']) && empty($element['#error_no_message'])) { + $variables['errors'] = $element['#errors']; + } + // If #title is not set, we don't display any label. if (!isset($element['#title'])) { $element['#title_display'] = 'none'; diff --git a/core/includes/theme.inc b/core/includes/theme.inc index 121da17467d799809d22999b05b04a6955e15e33..9ea8f05c34157c2e95062e097a16af720a1dcb8c 100644 --- a/core/includes/theme.inc +++ b/core/includes/theme.inc @@ -500,6 +500,12 @@ function template_preprocess_datetime_wrapper(&$variables) { $variables['title'] = $element['#title']; } + // Display any error messages. + $variables['errors'] = NULL; + if (!empty($element['#errors']) && empty($element['#error_no_message'])) { + $variables['errors'] = $element['#errors']; + } + if (!empty($element['#description'])) { $variables['description'] = $element['#description']; } diff --git a/core/lib/Drupal/Core/Datetime/Element/Datelist.php b/core/lib/Drupal/Core/Datetime/Element/Datelist.php index c330a4f8cbc8060e3253f28ee5673da99a21c14d..a0060ed843760b2dc56af69e8fe9a4500e368dd5 100644 --- a/core/lib/Drupal/Core/Datetime/Element/Datelist.php +++ b/core/lib/Drupal/Core/Datetime/Element/Datelist.php @@ -265,6 +265,7 @@ public static function processDatelist(&$element, FormStateInterface $form_state '#attributes' => $element['#attributes'], '#options' => $options, '#required' => $element['#required'], + '#error_no_message' => TRUE, ); } diff --git a/core/lib/Drupal/Core/Datetime/Element/Datetime.php b/core/lib/Drupal/Core/Datetime/Element/Datetime.php index d298e216bb5201359990b3d405828eb551e9baad..b8e4b6990523e357c7c68e789b379dec70029273 100644 --- a/core/lib/Drupal/Core/Datetime/Element/Datetime.php +++ b/core/lib/Drupal/Core/Datetime/Element/Datetime.php @@ -267,6 +267,7 @@ public static function processDatetime(&$element, FormStateInterface $form_state '#attributes' => $element['#attributes'] + $extra_attributes, '#required' => $element['#required'], '#size' => max(12, strlen($element['#value']['date'])), + '#error_no_message' => TRUE, ); // Allows custom callbacks to alter the element. @@ -298,6 +299,7 @@ public static function processDatetime(&$element, FormStateInterface $form_state '#attributes' => $element['#attributes'] + $extra_attributes, '#required' => $element['#required'], '#size' => 12, + '#error_no_message' => TRUE, ); // Allows custom callbacks to alter the element. diff --git a/core/lib/Drupal/Core/Form/FormElementHelper.php b/core/lib/Drupal/Core/Form/FormElementHelper.php new file mode 100644 index 0000000000000000000000000000000000000000..013eaa17c9a95657cb0b89e0a748d030d0e7b932 --- /dev/null +++ b/core/lib/Drupal/Core/Form/FormElementHelper.php @@ -0,0 +1,68 @@ +stringTranslation = $string_translation; + $this->linkGenerator = $link_generator; + } + + /** + * {@inheritdoc} + */ + public function handleFormErrors(array &$form, FormStateInterface $form_state) { + // After validation check if there are errors. + if ($errors = $form_state->getErrors()) { + // Display error messages for each element. + $this->displayErrorMessages($form, $form_state); + + // Loop through and assign each element its errors. + $this->setElementErrorsFromFormState($form, $form_state); + } + + return $this; + } + + /** + * Loops through and displays all form errors. + * + * @param array $form + * An associative array containing the structure of the form. + * @param \Drupal\Core\Form\FormStateInterface $form_state + * The current state of the form. + */ + protected function displayErrorMessages(array $form, FormStateInterface $form_state) { + $error_links = []; + $errors = $form_state->getErrors(); + // Loop through all form errors and check if we need to display a link. + foreach ($errors as $name => $error) { + $form_element = FormElementHelper::getElementByName($name, $form); + $title = FormElementHelper::getElementTitle($form_element); + + // Only show links to erroneous elements that are visible. + $is_visible_element = Element::isVisibleElement($form_element); + // Only show links for elements that have a title themselves or have + // children with a title. + $has_title = !empty($title); + // Only show links for elements with an ID. + $has_id = !empty($form_element['#id']); + + // Do not show links to elements with suppressed messages. Most often + // their parent element is used for inline errors. + if (!empty($form_element['#error_no_message'])) { + unset($errors[$name]); + } + elseif ($is_visible_element && $has_title && $has_id) { + // We need to pass this through SafeMarkup::escape() so + // drupal_set_message() does not encode the links. + $error_links[] = SafeMarkup::escape($this->l($title, Url::fromRoute('', [], ['fragment' => $form_element['#id'], 'external' => TRUE]))); + unset($errors[$name]); + } + } + + // Set normal error messages for all remaining errors. + foreach ($errors as $error) { + $this->drupalSetMessage($error, 'error'); + } + + if (!empty($error_links)) { + $message = $this->formatPlural(count($error_links), '1 error has been found: !errors', '@count errors have been found: !errors', [ + '!errors' => SafeMarkup::set(implode(', ', $error_links)), + ]); + $this->drupalSetMessage($message, 'error'); + } + } + + /** + * Stores the errors of each element directly on the element. + * + * We must provide a way for non-form functions to check the errors for a + * specific element. The most common usage of this is a #pre_render callback. + * + * @param array $elements + * An associative array containing the structure of a form element. + * @param \Drupal\Core\Form\FormStateInterface $form_state + * The current state of the form. + */ + protected function setElementErrorsFromFormState(array &$elements, FormStateInterface &$form_state) { + // Recurse through all children. + foreach (Element::children($elements) as $key) { + if (isset($elements[$key]) && $elements[$key]) { + $this->setElementErrorsFromFormState($elements[$key], $form_state); + } + } + + // Store the errors for this element on the element directly. + $elements['#errors'] = $form_state->getError($elements); + } + + /** + * Wraps drupal_set_message(). + * + * @codeCoverageIgnore + */ + protected function drupalSetMessage($message = NULL, $type = 'status', $repeat = FALSE) { + drupal_set_message($message, $type, $repeat); + } + +} diff --git a/core/lib/Drupal/Core/Form/FormErrorHandlerInterface.php b/core/lib/Drupal/Core/Form/FormErrorHandlerInterface.php new file mode 100644 index 0000000000000000000000000000000000000000..161505ff2982f597bdb7fe95bf594152f9602352 --- /dev/null +++ b/core/lib/Drupal/Core/Form/FormErrorHandlerInterface.php @@ -0,0 +1,27 @@ +errors = $errors; static::setAnyErrors(); - if ($message) { - $this->drupalSetMessage($message, 'error'); - } } } @@ -1119,7 +1116,7 @@ public function clearErrors() { * {@inheritdoc} */ public function getError(array $element) { - if ($errors = $this->getErrors($this)) { + if ($errors = $this->getErrors()) { $parents = array(); foreach ($element['#parents'] as $parent) { $parents[] = $parent; @@ -1244,15 +1241,6 @@ public function cleanValues() { return $this; } - /** - * Wraps drupal_set_message(). - * - * @return array|null - */ - protected function drupalSetMessage($message = NULL, $type = 'status', $repeat = FALSE) { - return drupal_set_message($message, $type, $repeat); - } - /** * Wraps ModuleHandler::loadInclude(). */ diff --git a/core/lib/Drupal/Core/Form/FormStateInterface.php b/core/lib/Drupal/Core/Form/FormStateInterface.php index 5e44ecb4bb1c663badfe138a7d22296d9f73fb3c..e0ca1c6a7aa241de058e863c8d640f9cf87967be 100644 --- a/core/lib/Drupal/Core/Form/FormStateInterface.php +++ b/core/lib/Drupal/Core/Form/FormStateInterface.php @@ -417,7 +417,8 @@ public static function hasAnyErrors(); * indicate which element needs to be changed and provide an error message. * This causes the Form API to not execute the form submit handlers, and * instead to re-display the form to the user with the corresponding elements - * rendered with an 'error' CSS class (shown as red by default). + * rendered with an 'error' CSS class (shown as red by default) and the error + * message near the element. * * The standard behavior of this method can be changed if a button provides * the #limit_validation_errors property. Multistep forms not wanting to diff --git a/core/lib/Drupal/Core/Form/FormValidator.php b/core/lib/Drupal/Core/Form/FormValidator.php index ee2da6518b8008393ce0a6becc676afce9594a5f..96f23203f8b0c7e1521f36b861d01d8a41856eaa 100644 --- a/core/lib/Drupal/Core/Form/FormValidator.php +++ b/core/lib/Drupal/Core/Form/FormValidator.php @@ -44,6 +44,13 @@ class FormValidator implements FormValidatorInterface { */ protected $logger; + /** + * The form error handler. + * + * @var \Drupal\Core\Form\FormErrorHandlerInterface + */ + protected $formErrorHandler; + /** * Constructs a new FormValidator. * @@ -55,12 +62,15 @@ class FormValidator implements FormValidatorInterface { * The CSRF token generator. * @param \Psr\Log\LoggerInterface $logger * A logger instance. + * @param \Drupal\Core\Form\FormErrorHandlerInterface $form_error_handler + * The form error handler. */ - public function __construct(RequestStack $request_stack, TranslationInterface $string_translation, CsrfTokenGenerator $csrf_token, LoggerInterface $logger) { + public function __construct(RequestStack $request_stack, TranslationInterface $string_translation, CsrfTokenGenerator $csrf_token, LoggerInterface $logger, FormErrorHandlerInterface $form_error_handler) { $this->requestStack = $request_stack; $this->stringTranslation = $string_translation; $this->csrfToken = $csrf_token; $this->logger = $logger; + $this->formErrorHandler = $form_error_handler; } /** @@ -184,8 +194,9 @@ protected function handleErrorsWithLimitedValidation(&$form, FormStateInterface * The unique string identifying the form. */ protected function finalizeValidation(&$form, FormStateInterface &$form_state, $form_id) { - // After validation, loop through and assign each element its errors. - $this->setElementErrorsFromFormState($form, $form_state); + // Delegate handling of form errors to a service. + $this->formErrorHandler->handleFormErrors($form, $form_state); + // Mark this form as validated. $form_state->setValidationComplete(); } @@ -394,26 +405,4 @@ protected function determineLimitValidationErrors(FormStateInterface &$form_stat } } - /** - * Stores the errors of each element directly on the element. - * - * We must provide a way for non-form functions to check the errors for a - * specific element. The most common usage of this is a #pre_render callback. - * - * @param array $elements - * An associative array containing the structure of a form element. - * @param \Drupal\Core\Form\FormStateInterface $form_state - * The current state of the form. - */ - protected function setElementErrorsFromFormState(array &$elements, FormStateInterface &$form_state) { - // Recurse through all children. - foreach (Element::children($elements) as $key) { - if (isset($elements[$key]) && $elements[$key]) { - $this->setElementErrorsFromFormState($elements[$key], $form_state); - } - } - // Store the errors for this element on the element directly. - $elements['#errors'] = $form_state->getError($elements); - } - } diff --git a/core/lib/Drupal/Core/Render/Element.php b/core/lib/Drupal/Core/Render/Element.php index c0d3a4a527b7efb640e1bff29f913eb5874ec7a1..67186f7e33de77c323a990be2c7c22ceb721f0b8 100644 --- a/core/lib/Drupal/Core/Render/Element.php +++ b/core/lib/Drupal/Core/Render/Element.php @@ -142,7 +142,7 @@ public static function getVisibleChildren(array $elements) { } // Skip value and hidden elements, since they are not rendered. - if (isset($child['#type']) && in_array($child['#type'], array('value', 'hidden'))) { + if (!static::isVisibleElement($child)) { continue; } @@ -152,6 +152,19 @@ public static function getVisibleChildren(array $elements) { return array_keys($visible_children); } + /** + * Determines if an element is visible. + * + * @param array $element + * The element to check for visibility. + * + * @return bool + * TRUE if the element is visible, otherwise FALSE. + */ + public static function isVisibleElement($element) { + return (!isset($element['#type']) || !in_array($element['#type'], ['value', 'hidden', 'token'])) && (!isset($element['#access']) || $element['#access']); + } + /** * Sets HTML attributes based on element properties. * diff --git a/core/lib/Drupal/Core/Render/Element/Checkboxes.php b/core/lib/Drupal/Core/Render/Element/Checkboxes.php index d9314606f9317d58160335d42f8fb92ac5168d30..117f839119f532b29da0b1945b5d2c573aea0c38 100644 --- a/core/lib/Drupal/Core/Render/Element/Checkboxes.php +++ b/core/lib/Drupal/Core/Render/Element/Checkboxes.php @@ -86,6 +86,8 @@ public static function processCheckboxes(&$element, FormStateInterface $form_sta '#default_value' => isset($value[$key]) ? $key : NULL, '#attributes' => $element['#attributes'], '#ajax' => isset($element['#ajax']) ? $element['#ajax'] : NULL, + // Errors should only be shown on the parent checkboxes element. + '#error_no_message' => TRUE, '#weight' => $weight, ); } diff --git a/core/lib/Drupal/Core/Render/Element/PasswordConfirm.php b/core/lib/Drupal/Core/Render/Element/PasswordConfirm.php index f7f6240d92ee7be97a60835c5cc228cda9dd9410..e7195d0f9d39384687335f6059060efb22cf9c41 100644 --- a/core/lib/Drupal/Core/Render/Element/PasswordConfirm.php +++ b/core/lib/Drupal/Core/Render/Element/PasswordConfirm.php @@ -26,6 +26,7 @@ public function getInfo() { $class = get_class($this); return array( '#input' => TRUE, + '#markup' => '', '#process' => array( array($class, 'processPasswordConfirm'), ), @@ -53,6 +54,7 @@ public static function processPasswordConfirm(&$element, FormStateInterface $for '#value' => empty($element['#value']) ? NULL : $element['#value']['pass1'], '#required' => $element['#required'], '#attributes' => array('class' => array('password-field', 'js-password-field')), + '#error_no_message' => TRUE, ); $element['pass2'] = array( '#type' => 'password', @@ -60,6 +62,7 @@ public static function processPasswordConfirm(&$element, FormStateInterface $for '#value' => empty($element['#value']) ? NULL : $element['#value']['pass2'], '#required' => $element['#required'], '#attributes' => array('class' => array('password-confirm', 'js-password-confirm')), + '#error_no_message' => TRUE, ); $element['#element_validate'] = array(array(get_called_class(), 'validatePasswordConfirm')); $element['#tree'] = TRUE; diff --git a/core/lib/Drupal/Core/Render/Element/Radios.php b/core/lib/Drupal/Core/Render/Element/Radios.php index 619c0b9f0396a4d51bb1f3626fb78c043f83db4e..5423ef03e9e4f8876f221cad6d7767fe64cbaed6 100644 --- a/core/lib/Drupal/Core/Render/Element/Radios.php +++ b/core/lib/Drupal/Core/Render/Element/Radios.php @@ -83,6 +83,8 @@ public static function processRadios(&$element, FormStateInterface $form_state, '#parents' => $element['#parents'], '#id' => HtmlUtility::getUniqueId('edit-' . implode('-', $parents_for_id)), '#ajax' => isset($element['#ajax']) ? $element['#ajax'] : NULL, + // Errors should only be shown on the parent radios element. + '#error_no_message' => TRUE, '#weight' => $weight, ); } diff --git a/core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php b/core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php index b613937bca496b15b88f3b13a94ad7e580df6382..e5243594c617ff33a7cc48a9fbe2472ac9b9fe0e 100644 --- a/core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php +++ b/core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php @@ -278,7 +278,7 @@ protected function doTestAuthoringInfo() { 'content_translation[created]' => '19/11/1978', ); $this->drupalPostForm($entity->urlInfo('edit-form'), $edit, $this->getFormSubmitAction($entity, $langcode)); - $this->assertTrue($this->xpath('//div[contains(@class, "error")]//ul'), 'Invalid values generate a list of form errors.'); + $this->assertTrue($this->xpath('//div[contains(concat(" ", normalize-space(@class), " "), :class)]', array(':class' => ' messages--error ')), 'Invalid values generate a form error message.'); $metadata = $this->manager->getTranslationMetadata($entity->getTranslation($langcode)); $this->assertEqual($metadata->getAuthor()->id(), $values[$langcode]['uid'], 'Translation author correctly kept.'); $this->assertEqual($metadata->getCreatedTime(), $values[$langcode]['created'], 'Translation date correctly kept.'); diff --git a/core/modules/file/file.module b/core/modules/file/file.module index 94ddc497de6c1897feb8d35219326ce59425ee99..7d36c58d5ee5c349c41501fc6980c771fa78fc9f 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -1163,7 +1163,7 @@ function file_managed_file_save_upload($element, FormStateInterface $form_state) $destination = isset($element['#upload_location']) ? $element['#upload_location'] : NULL; if (isset($destination) && !file_prepare_directory($destination, FILE_CREATE_DIRECTORY)) { \Drupal::logger('file')->notice('The upload directory %directory for the file field !name could not be created or is not accessible. A newly uploaded file could not be saved in this directory as a consequence, and the upload was canceled.', array('%directory' => $destination, '!name' => $element['#field_name'])); - $form_state->setErrorByName($upload_name, t('The file could not be uploaded.')); + $form_state->setError($element, t('The file could not be uploaded.')); return FALSE; } @@ -1173,7 +1173,7 @@ function file_managed_file_save_upload($element, FormStateInterface $form_state) if ($files_uploaded) { if (!$files = file_save_upload($upload_name, $element['#upload_validators'], $destination)) { \Drupal::logger('file')->notice('The file upload failed. %upload', array('%upload' => $upload_name)); - $form_state->setErrorByName($upload_name, t('Files in the !name field were unable to be uploaded.', array('!name' => $element['#title']))); + $form_state->setError($element, t('Files in the !name field were unable to be uploaded.', array('!name' => $element['#title']))); return array(); } diff --git a/core/modules/file/src/Element/ManagedFile.php b/core/modules/file/src/Element/ManagedFile.php index 40d0c3113014b68be9831b6abbae26f99b2c6abe..afde5af48890eb8ab81eed4f35882b2d2d406a41 100644 --- a/core/modules/file/src/Element/ManagedFile.php +++ b/core/modules/file/src/Element/ManagedFile.php @@ -233,6 +233,7 @@ public static function processManagedFile(&$element, FormStateInterface $form_st '#multiple' => $element['#multiple'], '#theme_wrappers' => [], '#weight' => -10, + '#error_no_message' => TRUE, ]; if (!empty($fids) && $element['#files']) { @@ -328,7 +329,7 @@ public static function validateManagedFile(&$element, FormStateInterface $form_s // Check required property based on the FID. if ($element['#required'] && empty($element['fids']['#value']) && !in_array($clicked_button, ['upload_button', 'remove_button'])) { - $form_state->setError($element['upload'], t('!name field is required.', ['!name' => $element['#title']])); + $form_state->setError($element, t('!name is required.', ['!name' => $element['#title']])); } // Consolidate the array value of this field to array of FIDs. diff --git a/core/modules/file/src/Tests/FileFieldValidateTest.php b/core/modules/file/src/Tests/FileFieldValidateTest.php index 51d740231d6cae17a7fc4dc0cf19d930c9a7c740..3e35a006d98989542d7663873e6e55dbe9e233b4 100644 --- a/core/modules/file/src/Tests/FileFieldValidateTest.php +++ b/core/modules/file/src/Tests/FileFieldValidateTest.php @@ -34,7 +34,8 @@ function testRequired() { $edit = array(); $edit['title[0][value]'] = $this->randomMachineName(); $this->drupalPostForm('node/add/' . $type_name, $edit, t('Save and publish')); - $this->assertRaw(t('!title field is required.', array('!title' => $field->getLabel())), 'Node save failed when required file field was empty.'); + $this->assertText('1 error has been found: ' . $field->label(), 'Node save failed when required file field was empty.'); + $this->assertIdentical(1, count($this->xpath('//div[contains(concat(" ", normalize-space(@class), " "), :class)]//a', [':class' => ' messages--error '])), 'There is one link in the error message.'); // Create a new node with the uploaded file. $nid = $this->uploadNodeFile($test_file, $field_name, $type_name); @@ -55,7 +56,8 @@ function testRequired() { $edit = array(); $edit['title[0][value]'] = $this->randomMachineName(); $this->drupalPostForm('node/add/' . $type_name, $edit, t('Save and publish')); - $this->assertRaw(t('!title field is required.', array('!title' => $field->getLabel())), 'Node save failed when required multiple value file field was empty.'); + $this->assertText('1 error has been found: ' . $field->label(), 'Node save failed when required multiple value file field was empty.'); + $this->assertIdentical(1, count($this->xpath('//div[contains(concat(" ", normalize-space(@class), " "), :class)]//a', [':class' => ' messages--error '])), 'There is one link in the error message.'); // Create a new node with the uploaded file into the multivalue field. $nid = $this->uploadNodeFile($test_file, $field_name, $type_name); diff --git a/core/modules/shortcut/src/Tests/ShortcutSetsTest.php b/core/modules/shortcut/src/Tests/ShortcutSetsTest.php index 56fce39422c991b8f2de00c94978bc44f5afcb30..db98745d9f54daacb3ac3a5e2c83cd4d4f694536 100644 --- a/core/modules/shortcut/src/Tests/ShortcutSetsTest.php +++ b/core/modules/shortcut/src/Tests/ShortcutSetsTest.php @@ -6,6 +6,8 @@ */ namespace Drupal\shortcut\Tests; + +use Drupal\Component\Utility\SafeMarkup; use Drupal\shortcut\Entity\ShortcutSet; /** @@ -131,7 +133,9 @@ function testShortcutSetSwitchCreate() { function testShortcutSetSwitchNoSetName() { $edit = array('set' => 'new'); $this->drupalPostForm('user/' . $this->adminUser->id() . '/shortcuts', $edit, t('Change set')); - $this->assertText(t('The new set label is required.')); + $this->assertRaw(\Drupal::translation()->formatPlural(1, '1 error has been found: !errors', '@count errors have been found: !errors', [ + '!errors' => SafeMarkup::set('Label') + ])); $current_set = shortcut_current_displayed_set($this->adminUser); $this->assertEqual($current_set->id(), $this->set->id(), 'Attempting to switch to a new shortcut set without providing a set name does not succeed.'); $this->assertFieldByXPath("//input[@name='label' and contains(concat(' ', normalize-space(@class), ' '), ' error ')]", NULL, 'The new set label field has the error class'); diff --git a/core/modules/system/css/system.theme.css b/core/modules/system/css/system.theme.css index c20d6caba8a7e3bfe33bace08fd4a75929793377..fa7a75bce57c5930e787ed0cfb6c35724f9ae732 100644 --- a/core/modules/system/css/system.theme.css +++ b/core/modules/system/css/system.theme.css @@ -113,6 +113,17 @@ abbr.ajax-changed { margin-right: 0; } +/* Inline error messages. */ +.form-error-message:before { + content: ''; + display: inline-block; + height: 14px; + width: 14px; + vertical-align: sub; + background: url(../../../misc/icons/ea2800/error.svg) no-repeat; + background-size: contain; +} + /** * Inline items. */ diff --git a/core/modules/system/src/Tests/Form/FormTest.php b/core/modules/system/src/Tests/Form/FormTest.php index 321502d290f08c85b842f22146118bad6b3ed6e6..be1fc2680ffb247ad31e461f1b324c3b56102ce8 100644 --- a/core/modules/system/src/Tests/Form/FormTest.php +++ b/core/modules/system/src/Tests/Form/FormTest.php @@ -188,7 +188,7 @@ function testRequiredCheckboxesRadio() { } // Check the page for error messages. - $errors = $this->xpath('//div[contains(@class, "error")]//li'); + $errors = $this->xpath('//div[contains(@class, "form-error-message")]//strong'); foreach ($errors as $error) { $expected_key = array_search($error[0], $expected); // If the error message is not one of the expected messages, fail. diff --git a/core/modules/system/src/Tests/Form/TriggeringElementProgrammedUnitTest.php b/core/modules/system/src/Tests/Form/TriggeringElementProgrammedUnitTest.php index ae6b82c6b30ba58e43bc1836bd2b0a6ba767822f..f7cffa1b2875deef5e72b52cdaffac9815354749 100644 --- a/core/modules/system/src/Tests/Form/TriggeringElementProgrammedUnitTest.php +++ b/core/modules/system/src/Tests/Form/TriggeringElementProgrammedUnitTest.php @@ -19,8 +19,20 @@ */ class TriggeringElementProgrammedUnitTest extends KernelTestBase implements FormInterface { + /** + * {@inheritdoc} + */ public static $modules = array('system'); + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + $this->installSchema('system', ['router']); + \Drupal::service('router.builder')->rebuild(); + } + /** * {@inheritdoc} */ diff --git a/core/modules/system/src/Tests/Form/ValidationTest.php b/core/modules/system/src/Tests/Form/ValidationTest.php index 7b6c9e84234cdcffea7de7ee21d196026a8f2225..eda6dae73dc433e1ef6d488bf4c3a778b1cbc9d1 100644 --- a/core/modules/system/src/Tests/Form/ValidationTest.php +++ b/core/modules/system/src/Tests/Form/ValidationTest.php @@ -7,7 +7,9 @@ namespace Drupal\system\Tests\Form; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Render\Element; +use Drupal\Core\Url; use Drupal\simpletest\WebTestBase; /** @@ -206,17 +208,33 @@ function testCustomRequiredError() { $edit = array(); $this->drupalPostForm('form-test/validate-required', $edit, 'Submit'); + $messages = []; foreach (Element::children($form) as $key) { if (isset($form[$key]['#required_error'])) { $this->assertNoText(t('!name field is required.', array('!name' => $form[$key]['#title']))); - $this->assertText($form[$key]['#required_error']); + $messages[] = [ + 'title' => $form[$key]['#title'], + 'message' => $form[$key]['#required_error'], + 'key' => $key, + ]; } elseif (isset($form[$key]['#form_test_required_error'])) { $this->assertNoText(t('!name field is required.', array('!name' => $form[$key]['#title']))); - $this->assertText($form[$key]['#form_test_required_error']); + $messages[] = [ + 'title' => $form[$key]['#title'], + 'message' => $form[$key]['#form_test_required_error'], + 'key' => $key, + ]; + } + elseif (!empty($form[$key]['#required'])) { + $messages[] = [ + 'title' => $form[$key]['#title'], + 'message' => t('!name field is required.', ['!name' => $form[$key]['#title']]), + 'key' => $key, + ]; } } - $this->assertNoText(t('An illegal choice has been detected. Please contact the site administrator.')); + $this->assertErrorMessages($messages); // Verify that no custom validation error appears with valid values. $edit = array( @@ -226,6 +244,7 @@ function testCustomRequiredError() { ); $this->drupalPostForm('form-test/validate-required', $edit, 'Submit'); + $messages = []; foreach (Element::children($form) as $key) { if (isset($form[$key]['#required_error'])) { $this->assertNoText(t('!name field is required.', array('!name' => $form[$key]['#title']))); @@ -235,7 +254,49 @@ function testCustomRequiredError() { $this->assertNoText(t('!name field is required.', array('!name' => $form[$key]['#title']))); $this->assertNoText($form[$key]['#form_test_required_error']); } + elseif (!empty($form[$key]['#required'])) { + $messages[] = [ + 'title' => $form[$key]['#title'], + 'message' => t('!name field is required.', ['!name' => $form[$key]['#title']]), + 'key' => $key, + ]; + } + } + $this->assertErrorMessages($messages); + } + + /** + * Asserts that the given error messages are displayed. + * + * @param array $messages + * An associative array of error messages keyed by the order they appear on + * the page, with the following key-value pairs: + * - title: The human readable form element title. + * - message: The error message for this form element. + * - key: The key used for the form element. + */ + protected function assertErrorMessages($messages) { + $element = $this->xpath('//div[@class = "form-error-message"]/strong'); + $this->assertIdentical(count($messages), count($element)); + + $error_links = []; + foreach ($messages as $delta => $message) { + // Ensure the message appears in the correct place. + if (!isset($element[$delta])) { + $this->fail(format_string('The error message for the "@title" element with key "@key" was not found.', ['@title' => $message['title'], '@key' => $message['key']])); + } + else { + $this->assertIdentical($message['message'], (string) $element[$delta]); + } + + // Gather the element for checking the jump link section. + $error_links[] = \Drupal::l($message['title'], Url::fromRoute('', [], ['fragment' => 'edit-' . str_replace('_', '-', $message['key']), 'external' => TRUE])); } + $top_message = \Drupal::translation()->formatPlural(count($error_links), '1 error has been found: !errors', '@count errors have been found: !errors', [ + '!errors' => SafeMarkup::set(implode(', ', $error_links)) + ]); + $this->assertRaw($top_message); $this->assertNoText(t('An illegal choice has been detected. Please contact the site administrator.')); } + } diff --git a/core/modules/system/templates/datetime-wrapper.html.twig b/core/modules/system/templates/datetime-wrapper.html.twig index d5418df95687d813278a27138e1957ef45a325ad..bb4b3aa34ce5fc6e5c99767e748b4c8638980190 100644 --- a/core/modules/system/templates/datetime-wrapper.html.twig +++ b/core/modules/system/templates/datetime-wrapper.html.twig @@ -24,4 +24,9 @@ {{ title }} {% endif %} {{ content }} +{% if errors %} +
+ {{ errors }} +
+{% endif %} {{ description }} diff --git a/core/modules/system/templates/fieldset.html.twig b/core/modules/system/templates/fieldset.html.twig index ab6796ca6f5c5fc125542bc015cc9118afcfbce0..aace95077d4206dcf07b4a856c6a445b28405156 100644 --- a/core/modules/system/templates/fieldset.html.twig +++ b/core/modules/system/templates/fieldset.html.twig @@ -5,6 +5,7 @@ * * Available variables: * - attributes: HTML attributes for the fieldset element. + * - errors: (optional) Any errors for this fieldset element, may not be set. * - required: Boolean indicating whether the fieldeset element is required. * - legend: The legend element containing the following properties: * - title: Title of the fieldset, intended for use as the text of the legend. @@ -33,6 +34,11 @@ {{ legend.title }}
+ {% if errors %} +
+ {{ errors }} +
+ {% endif %} {% if prefix %} {{ prefix }} {% endif %} diff --git a/core/modules/system/templates/form-element.html.twig b/core/modules/system/templates/form-element.html.twig index dbbb8301641ea9a11c8ea22b1957f64807d207fe..b1ca6e9ef9b1921bc9655c7f22c276e67c0afbbd 100644 --- a/core/modules/system/templates/form-element.html.twig +++ b/core/modules/system/templates/form-element.html.twig @@ -5,6 +5,7 @@ * * Available variables: * - attributes: HTML attributes for the containing element. + * - errors: (optional) Any errors for this form element, may not be set. * - prefix: (optional) The form element prefix, may not be set. * - suffix: (optional) The form element suffix, may not be set. * - required: The required marker, or empty if the associated form element is @@ -52,6 +53,7 @@ 'form-item-' ~ name|clean_class, title_display not in ['after', 'before'] ? 'form-no-label', disabled == 'disabled' ? 'form-disabled', + errors ? 'form-error', ] %} {% @@ -79,6 +81,11 @@ {% if label_display == 'after' %} {{ label }} {% endif %} + {% if errors %} +
+ {{ errors }} +
+ {% endif %} {% if description_display in ['after', 'invisible'] and description.content %} {{ description.content }} diff --git a/core/modules/user/src/Tests/UserBlocksTest.php b/core/modules/user/src/Tests/UserBlocksTest.php index 13492643e8e85f5a02e779bd3d75c62530bff3a8..366f51950cb60da5cd381f156df85570deadd60f 100644 --- a/core/modules/user/src/Tests/UserBlocksTest.php +++ b/core/modules/user/src/Tests/UserBlocksTest.php @@ -7,6 +7,7 @@ namespace Drupal\user\Tests; +use Drupal\Component\Utility\SafeMarkup; use Drupal\simpletest\WebTestBase; /** @@ -43,6 +44,16 @@ protected function setUp() { * Test the user login block. */ function testUserLoginBlock() { + // Make sure the validation error is displayed when try to login with + // invalid username/password. + $edit['name'] = $this->randomMachineName(); + $edit['pass'] = $this->randomMachineName(); + $this->drupalPostForm('node', $edit, t('Log in')); + $this->assertRaw(\Drupal::translation()->formatPlural(1, '1 error has been found: !errors', '@count errors have been found: !errors', [ + '!errors' => SafeMarkup::set('Username') + ])); + $this->assertText(t('Sorry, unrecognized username or password.')); + // Create a user with some permission that anonymous users lack. $user = $this->drupalCreateUser(array('administer permissions')); diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index 825bd8a8b76762bdac1e16dae7689166b4921ba9..77c8763c69a992531162538f6da165dcecbcb89d 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -370,15 +370,11 @@ public function testUniqueHtmlId() { ->method('buildForm') ->will($this->returnValue($expected_form)); - $form_state = $this->getMockBuilder('Drupal\Core\Form\FormState') - ->setMethods(array('drupalSetMessage')) - ->getMock(); + $form_state = new FormState(); $form = $this->simulateFormSubmission($form_id, $form_arg, $form_state); $this->assertSame('test-form-id', $form['#id']); - $form_state = $this->getMockBuilder('Drupal\Core\Form\FormState') - ->setMethods(array('drupalSetMessage')) - ->getMock(); + $form_state = new FormState(); $form = $this->simulateFormSubmission($form_id, $form_arg, $form_state); $this->assertSame('test-form-id--2', $form['#id']); } diff --git a/core/tests/Drupal/Tests/Core/Form/FormElementHelperTest.php b/core/tests/Drupal/Tests/Core/Form/FormElementHelperTest.php new file mode 100644 index 0000000000000000000000000000000000000000..d25a6f5429e86e935f31f7dc46045f8a3165f589 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Form/FormElementHelperTest.php @@ -0,0 +1,179 @@ +assertSame($expected, FormElementHelper::getElementByName($name, $form)); + } + + /** + * Provides test data. + */ + public function getElementByNameProvider() { + $data = []; + $data[] = ['id', [], []]; + $data[] = [ + 'id', + [ + 'id' => [ + '#title' => 'ID', + '#parents' => ['id'], + ], + ], + [ + '#title' => 'ID', + '#parents' => ['id'], + ], + ]; + $data[] = [ + 'id', + [ + 'fieldset' => [ + 'id' => [ + '#title' => 'ID', + '#parents' => ['id'], + ], + '#parents' => ['fieldset'], + ], + ], + [ + '#title' => 'ID', + '#parents' => ['id'], + ], + ]; + $data[] = [ + 'fieldset', + [ + 'fieldset' => [ + 'id' => [ + '#title' => 'ID', + '#parents' => ['id'], + ], + '#parents' => ['fieldset'], + ], + ], + [ + 'id' => [ + '#title' => 'ID', + '#parents' => ['id'], + ], + '#parents' => ['fieldset'], + ], + ]; + $data[] = [ + 'fieldset][id', + [ + 'fieldset' => [ + '#tree' => TRUE, + 'id' => [ + '#title' => 'ID', + '#parents' => ['fieldset', 'id'], + ], + '#parents' => ['fieldset'], + ], + ], + [ + '#title' => 'ID', + '#parents' => ['fieldset', 'id'], + ], + ]; + return $data; + } + + /** + * Tests the getElementTitle() method. + * + * @covers ::getElementTitle + * + * @dataProvider getElementTitleProvider + */ + public function testGetElementTitle($name, $form, $expected) { + $element = FormElementHelper::getElementByName($name, $form); + $this->assertSame($expected, FormElementHelper::getElementTitle($element)); + } + + /** + * Provides test data. + */ + public function getElementTitleProvider() { + $data = []; + $data[] = ['id', [], '']; + $data[] = [ + 'id', + [ + 'id' => [ + '#title' => 'ID', + '#parents' => ['id'], + ], + ], + 'ID', + ]; + $data[] = [ + 'id', + [ + 'fieldset' => [ + 'id' => [ + '#title' => 'ID', + '#parents' => ['id'], + ], + '#parents' => ['fieldset'], + ], + ], + 'ID', + ]; + $data[] = [ + 'fieldset', + [ + 'fieldset' => [ + 'id' => [ + '#title' => 'ID', + '#parents' => ['id'], + ], + '#parents' => ['fieldset'], + ], + ], + 'ID', + ]; + $data[] = [ + 'fieldset][id', + [ + 'fieldset' => [ + '#tree' => TRUE, + 'id' => [ + '#title' => 'ID', + '#parents' => ['fieldset', 'id'], + ], + '#parents' => ['fieldset'], + ], + ], + 'ID', + ]; + return $data; + } + +} diff --git a/core/tests/Drupal/Tests/Core/Form/FormErrorHandlerTest.php b/core/tests/Drupal/Tests/Core/Form/FormErrorHandlerTest.php new file mode 100644 index 0000000000000000000000000000000000000000..53d7e76559788bc070a86727d045aadd865560bc --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Form/FormErrorHandlerTest.php @@ -0,0 +1,125 @@ +getMock('Drupal\Core\Utility\LinkGeneratorInterface'); + $link_generator->expects($this->any()) + ->method('generate') + ->willReturnArgument(0); + $form_error_handler = $this->getMockBuilder('Drupal\Core\Form\FormErrorHandler') + ->setConstructorArgs([$this->getStringTranslationStub(), $link_generator]) + ->setMethods(['drupalSetMessage']) + ->getMock(); + + $form_error_handler->expects($this->at(0)) + ->method('drupalSetMessage') + ->with('no title given', 'error'); + $form_error_handler->expects($this->at(1)) + ->method('drupalSetMessage') + ->with('element is invisible', 'error'); + $form_error_handler->expects($this->at(2)) + ->method('drupalSetMessage') + ->with('this missing element is invalid', 'error'); + $form_error_handler->expects($this->at(3)) + ->method('drupalSetMessage') + ->with('3 errors have been found: Test 1, Test 2 & a half, Test 3', 'error'); + + $form = [ + '#parents' => [], + ]; + $form['test1'] = [ + '#type' => 'textfield', + '#title' => 'Test 1', + '#parents' => ['test1'], + '#id' => 'edit-test1', + ]; + $form['test2'] = [ + '#type' => 'textfield', + '#title' => 'Test 2 & a half', + '#parents' => ['test2'], + '#id' => 'edit-test2', + ]; + $form['fieldset'] = [ + '#parents' => ['fieldset'], + 'test3' => [ + '#type' => 'textfield', + '#title' => 'Test 3', + '#parents' => ['fieldset', 'test3'], + '#id' => 'edit-test3', + ], + ]; + $form['test4'] = [ + '#type' => 'textfield', + '#title' => 'Test 4', + '#parents' => ['test4'], + '#id' => 'edit-test4', + '#error_no_message' => TRUE, + ]; + $form['test5'] = [ + '#type' => 'textfield', + '#parents' => ['test5'], + '#id' => 'edit-test5', + ]; + $form['test6'] = [ + '#type' => 'value', + '#title' => 'Test 6', + '#parents' => ['test6'], + '#id' => 'edit-test6', + ]; + $form_state = new FormState(); + $form_state->setErrorByName('test1', 'invalid'); + $form_state->setErrorByName('test2', 'invalid'); + $form_state->setErrorByName('fieldset][test3', 'invalid'); + $form_state->setErrorByName('test4', 'no error message'); + $form_state->setErrorByName('test5', 'no title given'); + $form_state->setErrorByName('test6', 'element is invisible'); + $form_state->setErrorByName('missing_element', 'this missing element is invalid'); + $form_error_handler->handleFormErrors($form, $form_state); + $this->assertSame('invalid', $form['test1']['#errors']); + } + + /** + * @covers ::handleFormErrors + * @covers ::setElementErrorsFromFormState + */ + public function testSetElementErrorsFromFormState() { + $form_error_handler = $this->getMockBuilder('Drupal\Core\Form\FormErrorHandler') + ->setConstructorArgs([$this->getStringTranslationStub(), $this->getMock('Drupal\Core\Utility\LinkGeneratorInterface')]) + ->setMethods(['drupalSetMessage']) + ->getMock(); + + $form = [ + '#parents' => [], + ]; + $form['test'] = [ + '#type' => 'textfield', + '#title' => 'Test', + '#parents' => ['test'], + '#id' => 'edit-test', + ]; + $form_state = new FormState(); + $form_state->setErrorByName('test', 'invalid'); + $form_error_handler->handleFormErrors($form, $form_state); + $this->assertSame('invalid', $form['test']['#errors']); + } + +} diff --git a/core/tests/Drupal/Tests/Core/Form/FormStateTest.php b/core/tests/Drupal/Tests/Core/Form/FormStateTest.php index 24c4ddd28b5c78a0b51bf82fc416bcec34efff1e..b92c5f7b86be934f290fcf78caf1f0c58a6c6cd8 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormStateTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormStateTest.php @@ -62,15 +62,10 @@ public function providerTestGetRedirect() { * @covers ::setError */ public function testSetError() { - $form_state = $this->getMockBuilder('Drupal\Core\Form\FormState') - ->setMethods(array('drupalSetMessage')) - ->getMock(); - $form_state->expects($this->once()) - ->method('drupalSetMessage') - ->willReturn('Fail'); - + $form_state = new FormState(); $element['#parents'] = array('foo', 'bar'); $form_state->setError($element, 'Fail'); + $this->assertSame(['foo][bar' => 'Fail'], $form_state->getErrors()); } /** @@ -108,14 +103,10 @@ public function providerTestGetError() { * * @dataProvider providerTestSetErrorByName */ - public function testSetErrorByName($limit_validation_errors, $expected_errors, $set_message = FALSE) { - $form_state = $this->getMockBuilder('Drupal\Core\Form\FormState') - ->setMethods(array('drupalSetMessage')) - ->getMock(); + public function testSetErrorByName($limit_validation_errors, $expected_errors) { + $form_state = new FormState(); $form_state->setLimitValidationErrors($limit_validation_errors); $form_state->clearErrors(); - $form_state->expects($set_message ? $this->once() : $this->never()) - ->method('drupalSetMessage'); $form_state->setErrorByName('test', 'Fail 1'); $form_state->setErrorByName('test', 'Fail 2'); @@ -131,7 +122,7 @@ public function providerTestSetErrorByName() { array(array(array('options')), array('options' => '')), // Do not limit an validation, and, ensuring the first error is returned // for the 'test' element. - array(NULL, array('test' => 'Fail 1', 'options' => ''), TRUE), + [NULL, ['test' => 'Fail 1', 'options' => '']], // Limit all validation. array(array(), array()), ); @@ -146,9 +137,7 @@ public function providerTestSetErrorByName() { * @expectedExceptionMessage Form errors cannot be set after form validation has finished. */ public function testFormErrorsDuringSubmission() { - $form_state = $this->getMockBuilder('Drupal\Core\Form\FormState') - ->setMethods(array('drupalSetMessage')) - ->getMock(); + $form_state = new FormState(); $form_state->setValidationComplete(); $form_state->setErrorByName('test', 'message'); } diff --git a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php index 12b32609d39794f1f512926ef78ac74a7cd979d0..b6fe9dd923f53f9e45582fc4f033a385e2927e23 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php +++ b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php @@ -178,9 +178,10 @@ protected function setUp() { $this->requestStack = new RequestStack(); $this->requestStack->push($this->request); $this->logger = $this->getMock('Drupal\Core\Logger\LoggerChannelInterface'); + $form_error_handler = $this->getMock('Drupal\Core\Form\FormErrorHandlerInterface'); $this->formValidator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->setConstructorArgs(array($this->requestStack, $this->getStringTranslationStub(), $this->csrfToken, $this->logger)) - ->setMethods(array('drupalSetMessage')) + ->setConstructorArgs([$this->requestStack, $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $form_error_handler]) + ->setMethods(NULL) ->getMock(); $this->formSubmitter = $this->getMockBuilder('Drupal\Core\Form\FormSubmitter') ->setConstructorArgs(array($this->requestStack, $this->urlGenerator)) diff --git a/core/tests/Drupal/Tests/Core/Form/FormValidatorTest.php b/core/tests/Drupal/Tests/Core/Form/FormValidatorTest.php index 5165c0c7168f94e631593d017617b1b82dd84de8..d701991b059debcf341f11f0763abe81d983503a 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormValidatorTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormValidatorTest.php @@ -19,6 +19,39 @@ */ class FormValidatorTest extends UnitTestCase { + /** + * A logger instance. + * + * @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $logger; + + /** + * The CSRF token generator to validate the form token. + * + * @var \Drupal\Core\Access\CsrfTokenGenerator|\PHPUnit_Framework_MockObject_MockObject + */ + protected $csrfToken; + + /** + * The form error handler. + * + * @var \Drupal\Core\Form\FormErrorHandlerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $formErrorHandler; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + $this->logger = $this->getMock('Psr\Log\LoggerInterface'); + $this->csrfToken = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') + ->disableOriginalConstructor() + ->getMock(); + $this->formErrorHandler = $this->getMock('Drupal\Core\Form\FormErrorHandlerInterface'); + } + /** * Tests the 'validation_complete' $form_state flag. * @@ -27,7 +60,7 @@ class FormValidatorTest extends UnitTestCase { */ public function testValidationComplete() { $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->disableOriginalConstructor() + ->setConstructorArgs([new RequestStack(), $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $this->formErrorHandler]) ->setMethods(NULL) ->getMock(); @@ -45,7 +78,7 @@ public function testValidationComplete() { */ public function testPreventDuplicateValidation() { $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->disableOriginalConstructor() + ->setConstructorArgs([new RequestStack(), $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $this->formErrorHandler]) ->setMethods(array('doValidateForm')) ->getMock(); $form_validator->expects($this->never()) @@ -65,18 +98,19 @@ public function testPreventDuplicateValidation() { */ public function testMustValidate() { $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->disableOriginalConstructor() + ->setConstructorArgs([new RequestStack(), $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $this->formErrorHandler]) ->setMethods(array('doValidateForm')) ->getMock(); $form_validator->expects($this->once()) ->method('doValidateForm'); + $this->formErrorHandler->expects($this->once()) + ->method('handleFormErrors'); $form = array(); $form_state = (new FormState()) ->setValidationComplete() ->setValidationEnforced(); $form_validator->validateForm('test_form_id', $form, $form_state); - $this->assertArrayHasKey('#errors', $form); } /** @@ -86,16 +120,12 @@ public function testValidateInvalidFormToken() { $request_stack = new RequestStack(); $request = new Request(array(), array(), array(), array(), array(), array('REQUEST_URI' => '/test/example?foo=bar')); $request_stack->push($request); - $csrf_token = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') - ->disableOriginalConstructor() - ->getMock(); - $csrf_token->expects($this->once()) + $this->csrfToken->expects($this->once()) ->method('validate') ->will($this->returnValue(FALSE)); - $logger = $this->getMock('Psr\Log\LoggerInterface'); $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->setConstructorArgs(array($request_stack, $this->getStringTranslationStub(), $csrf_token, $logger)) + ->setConstructorArgs([$request_stack, $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $this->formErrorHandler]) ->setMethods(array('doValidateForm')) ->getMock(); $form_validator->expects($this->never()) @@ -118,16 +148,12 @@ public function testValidateInvalidFormToken() { */ public function testValidateValidFormToken() { $request_stack = new RequestStack(); - $csrf_token = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') - ->disableOriginalConstructor() - ->getMock(); - $csrf_token->expects($this->once()) + $this->csrfToken->expects($this->once()) ->method('validate') ->will($this->returnValue(TRUE)); - $logger = $this->getMock('Psr\Log\LoggerInterface'); $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->setConstructorArgs(array($request_stack, $this->getStringTranslationStub(), $csrf_token, $logger)) + ->setConstructorArgs([$request_stack, $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $this->formErrorHandler]) ->setMethods(array('doValidateForm')) ->getMock(); $form_validator->expects($this->once()) @@ -144,31 +170,6 @@ public function testValidateValidFormToken() { $this->assertTrue($form_state->isValidationComplete()); } - /** - * @covers ::setElementErrorsFromFormState - */ - public function testSetElementErrorsFromFormState() { - $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->disableOriginalConstructor() - ->setMethods(NULL) - ->getMock(); - - $form = array( - '#parents' => array(), - ); - $form['test'] = array( - '#type' => 'textfield', - '#title' => 'Test', - '#parents' => array('test'), - ); - $form_state = $this->getMockBuilder('Drupal\Core\Form\FormState') - ->setMethods(array('drupalSetMessage')) - ->getMock(); - $form_state->setErrorByName('test', 'invalid'); - $form_validator->validateForm('test_form_id', $form, $form_state); - $this->assertSame('invalid', $form['test']['#errors']); - } - /** * @covers ::handleErrorsWithLimitedValidation * @@ -176,7 +177,7 @@ public function testSetElementErrorsFromFormState() { */ public function testHandleErrorsWithLimitedValidation($sections, $triggering_element, $values, $expected) { $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->disableOriginalConstructor() + ->setConstructorArgs([new RequestStack(), $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $this->formErrorHandler]) ->setMethods(NULL) ->getMock(); @@ -272,7 +273,7 @@ public function providerTestHandleErrorsWithLimitedValidation() { */ public function testExecuteValidateHandlers() { $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->disableOriginalConstructor() + ->setConstructorArgs([new RequestStack(), $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $this->formErrorHandler]) ->setMethods(NULL) ->getMock(); $mock = $this->getMock('stdClass', array('validate_handler', 'hash_validate')); @@ -302,13 +303,8 @@ public function testExecuteValidateHandlers() { * @dataProvider providerTestRequiredErrorMessage */ public function testRequiredErrorMessage($element, $expected_message) { - $csrf_token = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') - ->disableOriginalConstructor() - ->getMock(); - $logger = $this->getMock('Psr\Log\LoggerInterface'); - $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->setConstructorArgs(array(new RequestStack(), $this->getStringTranslationStub(), $csrf_token, $logger)) + ->setConstructorArgs([new RequestStack(), $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $this->formErrorHandler]) ->setMethods(array('executeValidateHandlers')) ->getMock(); $form_validator->expects($this->once()) @@ -356,7 +352,7 @@ public function providerTestRequiredErrorMessage() { */ public function testElementValidate() { $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->disableOriginalConstructor() + ->setConstructorArgs([new RequestStack(), $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $this->formErrorHandler]) ->setMethods(array('executeValidateHandlers')) ->getMock(); $form_validator->expects($this->once()) @@ -383,18 +379,13 @@ public function testElementValidate() { * @dataProvider providerTestPerformRequiredValidation */ public function testPerformRequiredValidation($element, $expected_message, $call_watchdog) { - $csrf_token = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') - ->disableOriginalConstructor() - ->getMock(); - $logger = $this->getMock('Psr\Log\LoggerInterface'); - $form_validator = $this->getMockBuilder('Drupal\Core\Form\FormValidator') - ->setConstructorArgs(array(new RequestStack(), $this->getStringTranslationStub(), $csrf_token, $logger)) + ->setConstructorArgs([new RequestStack(), $this->getStringTranslationStub(), $this->csrfToken, $this->logger, $this->formErrorHandler]) ->setMethods(array('setError')) ->getMock(); if ($call_watchdog) { - $logger->expects($this->once()) + $this->logger->expects($this->once()) ->method('error') ->with($this->isType('string'), $this->isType('array')); } diff --git a/core/tests/Drupal/Tests/UnitTestCase.php b/core/tests/Drupal/Tests/UnitTestCase.php index 9e70a73b9a664ff7e351526068ba6013e3da2581..67f7bcd6731364d3b9bea606f1db7108936de39d 100644 --- a/core/tests/Drupal/Tests/UnitTestCase.php +++ b/core/tests/Drupal/Tests/UnitTestCase.php @@ -9,6 +9,7 @@ use Drupal\Component\FileCache\FileCacheFactory; use Drupal\Component\Utility\Random; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Cache\CacheTagsInvalidatorInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; @@ -208,6 +209,11 @@ public function getStringTranslationStub() { $translation->expects($this->any()) ->method('translate') ->will($this->returnCallback('Drupal\Component\Utility\SafeMarkup::format')); + $translation->expects($this->any()) + ->method('formatPlural') + ->willReturnCallback(function ($count, $singular, $plural, array $args = [], array $options = []) { + return $count === 1 ? SafeMarkup::format($singular, $args) : SafeMarkup::format($plural, $args + ['@count' => $count]); + }); return $translation; } diff --git a/core/themes/bartik/css/components/form.css b/core/themes/bartik/css/components/form.css index a5cfcf6731663859896dc8bbd5bf6d994f71b97e..4574da0cadc98873acd9410aeebf40886a82b5c0 100644 --- a/core/themes/bartik/css/components/form.css +++ b/core/themes/bartik/css/components/form.css @@ -269,3 +269,13 @@ input.form-submit:focus { margin-left: 0.6em; margin-right: 0; } + +/* Form error styles. */ +.form-item textarea.error + .cke { + border: 2px solid red; +} + +/* Form error message styles. */ +.form-error-message { + color: #ea2800; +} diff --git a/core/themes/classy/templates/form/datetime-wrapper.html.twig b/core/themes/classy/templates/form/datetime-wrapper.html.twig index 399dea2d68e6be9ff039aa9d64de152c3b8011f0..56da17309d6df1196c913aede453d5dfead38d86 100644 --- a/core/themes/classy/templates/form/datetime-wrapper.html.twig +++ b/core/themes/classy/templates/form/datetime-wrapper.html.twig @@ -23,6 +23,11 @@ {{ title }} {% endif %} {{ content }} +{% if errors %} +
+ {{ errors }} +
+{% endif %} {% if description %}
{{ description }}
{% endif %} diff --git a/core/themes/classy/templates/form/fieldset.html.twig b/core/themes/classy/templates/form/fieldset.html.twig index f7460cf824798173f96c9feb5256ea06a5dc3d14..92860742bc84e22d4a67823c2dc2ccda2c506e3a 100644 --- a/core/themes/classy/templates/form/fieldset.html.twig +++ b/core/themes/classy/templates/form/fieldset.html.twig @@ -5,6 +5,7 @@ * * Available variables: * - attributes: HTML attributes for the fieldset element. + * - errors: (optional) Any errors for this fieldset element, may not be set. * - required: Boolean indicating whether the fieldeset element is required. * - legend: The legend element containing the following properties: * - title: Title of the fieldset, intended for use as the text of the legend. @@ -31,6 +32,11 @@ {{ legend.title }}
+ {% if errors %} +
+ {{ errors }} +
+ {% endif %} {% if prefix %} {{ prefix }} {% endif %} diff --git a/core/themes/classy/templates/form/form-element.html.twig b/core/themes/classy/templates/form/form-element.html.twig index 5dc4000fb963ee65b8d7fa5a450c971c6dcbcf7c..f0d4d5601225dc4ca2914b17e46637c60a153f1d 100644 --- a/core/themes/classy/templates/form/form-element.html.twig +++ b/core/themes/classy/templates/form/form-element.html.twig @@ -5,6 +5,7 @@ * * Available variables: * - attributes: HTML attributes for the containing element. + * - errors: (optional) Any errors for this form element, may not be set. * - prefix: (optional) The form element prefix, may not be set. * - suffix: (optional) The form element suffix, may not be set. * - required: The required marker, or empty if the associated form element is @@ -51,6 +52,7 @@ 'form-item-' ~ name|clean_class, title_display not in ['after', 'before'] ? 'form-no-label', disabled == 'disabled' ? 'form-disabled', + errors ? 'form-error', ] %} {% @@ -78,6 +80,11 @@ {% if label_display == 'after' %} {{ label }} {% endif %} + {% if errors %} +
+ {{ errors }} +
+ {% endif %} {% if description_display in ['after', 'invisible'] and description.content %} {{ description.content }} diff --git a/core/themes/seven/css/components/form.css b/core/themes/seven/css/components/form.css index 5dd19be25f922feb08917fc5fb156753bad0ceda..07232de4f7636c9b38736a7e23e8706957c78d3b 100644 --- a/core/themes/seven/css/components/form.css +++ b/core/themes/seven/css/components/form.css @@ -55,7 +55,6 @@ label[for] { .form-disabled label { color: #737373; } - .form-disabled input.form-text, .form-disabled input.form-tel, .form-disabled input.form-email, @@ -70,16 +69,19 @@ label[for] { background-color: hsla(0, 0%, 0%, .08); box-shadow: none; } - .form-item input.error, .form-item textarea.error, .form-item select.error { - border-width: 2px; + border-width: 1px; border-color: #e62600; background-color: hsla(15, 75%, 97%, 1); box-shadow: inset 0 5px 5px -5px #b8b8b8; color: #a51b00; } +.form-item textarea.error + .cke { + border-width: 1px; + border-color: #e62600; +} .form-item input.error:focus, .form-item textarea.error:focus, .form-item select.error:focus { @@ -94,6 +96,19 @@ label[for] { width: 7px; height: 7px; } +.form-error-message { + margin-top: 0.15em; + color: #ea2800; +} +.fieldset-wrapper > .form-error-message { + margin-top: 0; +} +.text-format-wrapper .form-error-message { + border: solid #ccc; + border-width: 0 1px; + margin: 0; + padding: 0.25em 0.666em 0; +} /* Filter */ @@ -105,6 +120,7 @@ div.description, font-size: 0.95em; } .form-item .description.error { + margin-top: 0; color: #a51b00; } @@ -183,15 +199,6 @@ textarea.form-textarea { width: auto; } -.form-item .password-suggestions { - float: left; /* LTR */ - clear: left; /* LTR */ - width: 100%; -} -[dir="rtl"] .form-item .password-suggestions { - float: right; - clear: right; -} .form-item-pass .description { clear: both; }