diff --git a/core/core.services.yml b/core/core.services.yml index 14c665c..5619213 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -290,10 +290,13 @@ services: arguments: ['@form_validator', '@form_submitter', '@form_cache', '@module_handler', '@event_dispatcher', '@request_stack', '@class_resolver', '@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', '@config.factory', '@request_stack', '@page_cache_request_policy'] diff --git a/core/includes/form.inc b/core/includes/form.inc index 44a049c..95b8204 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; @@ -215,6 +216,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_use_parent'])) { + $variables['errors'] = $element['#errors']; + } } /** @@ -425,7 +432,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']; } @@ -441,6 +448,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_use_parent'])) { + $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/lib/Drupal/Core/Form/FormElementHelper.php b/core/lib/Drupal/Core/Form/FormElementHelper.php new file mode 100644 index 0000000..9d3e193 --- /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) { + // Display error messages for each element. + $this->displayErrorMessages($form, $form_state); + + // After validation, 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 = []; + // Loop through all form errors and display a link for each error that is + // associated with a visible form element. + foreach ($form_state->getErrors() as $key => $error) { + if (($form_element = FormElementHelper::getElementByName($key, $form)) && Element::isVisibleElement($form_element)) { + $title = FormElementHelper::getElementTitle($form_element); + $error_links[] = $this->l($title, Url::fromRoute('', [], ['fragment' => 'edit-' . str_replace('_', '-', $key), 'external' => TRUE])); + } + else { + $this->drupalSetMessage($error, 'error'); + } + } + + if (!empty($error_links)) { + // We need to pass this through SafeMarkup::format() so + // drupal_set_message() does not encode the links. + $message_error_links = ''; + $separator = ''; + foreach ($error_links as $error_link) { + $message_error_links .= $separator . SafeMarkup::escape($error_link); + $separator = ', '; + } + $message = $this->formatPlural(count($error_links), '1 error has been found: !errors', '@count errors have been found: !errors', [ + '!errors' => SafeMarkup::set($message_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 0000000..161505f --- /dev/null +++ b/core/lib/Drupal/Core/Form/FormErrorHandlerInterface.php @@ -0,0 +1,27 @@ +errors = $errors; static::setAnyErrors(); - if ($message) { - $this->drupalSetMessage($message, 'error'); - } } } @@ -1245,15 +1242,6 @@ public function cleanValues() { } /** - * 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(). */ protected function moduleLoadInclude($module, $type, $name = NULL) { diff --git a/core/lib/Drupal/Core/Form/FormValidator.php b/core/lib/Drupal/Core/Form/FormValidator.php index ee2da65..96f2320 100644 --- a/core/lib/Drupal/Core/Form/FormValidator.php +++ b/core/lib/Drupal/Core/Form/FormValidator.php @@ -45,6 +45,13 @@ class FormValidator implements FormValidatorInterface { protected $logger; /** + * The form error handler. + * + * @var \Drupal\Core\Form\FormErrorHandlerInterface + */ + protected $formErrorHandler; + + /** * Constructs a new FormValidator. * * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack @@ -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 f268a01..e34e002 100644 --- a/core/lib/Drupal/Core/Render/Element.php +++ b/core/lib/Drupal/Core/Render/Element.php @@ -131,7 +131,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; } @@ -142,6 +142,19 @@ public static function getVisibleChildren(array $elements) { } /** + * 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']); + } + + /** * Sets HTML attributes based on element properties. * * @param array $element diff --git a/core/lib/Drupal/Core/Render/Element/Checkboxes.php b/core/lib/Drupal/Core/Render/Element/Checkboxes.php index 1891ceb..f3a2872 100644 --- a/core/lib/Drupal/Core/Render/Element/Checkboxes.php +++ b/core/lib/Drupal/Core/Render/Element/Checkboxes.php @@ -74,6 +74,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_use_parent' => TRUE, '#weight' => $weight, ); } diff --git a/core/lib/Drupal/Core/Render/Element/PasswordConfirm.php b/core/lib/Drupal/Core/Render/Element/PasswordConfirm.php index 9bc32b7..dcdf5a8 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')), + '#error_use_parent' => TRUE, ); $element['pass2'] = array( '#type' => 'password', @@ -60,9 +62,11 @@ public static function processPasswordConfirm(&$element, FormStateInterface $for '#value' => empty($element['#value']) ? NULL : $element['#value']['pass2'], '#required' => $element['#required'], '#attributes' => array('class' => array('password-confirm')), + '#error_use_parent' => TRUE, ); $element['#element_validate'] = array(array(get_called_class(), 'validatePasswordConfirm')); $element['#tree'] = TRUE; + $element['#theme_wrappers'] = ['fieldset']; if (isset($element['#size'])) { $element['pass1']['#size'] = $element['pass2']['#size'] = $element['#size']; diff --git a/core/lib/Drupal/Core/Render/Element/Radios.php b/core/lib/Drupal/Core/Render/Element/Radios.php index 4b27b5a..ac9c563 100644 --- a/core/lib/Drupal/Core/Render/Element/Radios.php +++ b/core/lib/Drupal/Core/Render/Element/Radios.php @@ -68,6 +68,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_use_parent' => TRUE, '#weight' => $weight, ); } diff --git a/core/modules/shortcut/src/Tests/ShortcutSetsTest.php b/core/modules/shortcut/src/Tests/ShortcutSetsTest.php index 417e918..151c0a0 100644 --- a/core/modules/shortcut/src/Tests/ShortcutSetsTest.php +++ b/core/modules/shortcut/src/Tests/ShortcutSetsTest.php @@ -131,7 +131,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('New set') + ])); $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.'); } diff --git a/core/modules/system/css/system.theme.css b/core/modules/system/css/system.theme.css index f7af1e4..7a8d8d5 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. */ @@ -549,6 +560,7 @@ ul.tabs { background-image: url(../../../misc/icons/ea2800/error.svg); border-color: #f9c9bf #f9c9bf #f9c9bf transparent; /* LTR */ box-shadow: -8px 0 0 #e62600; /* LTR */ + margin-left: 8px; } [dir="rtl"] .messages--error { border-color: #f9c9bf transparent #f9c9bf #f9c9bf; diff --git a/core/modules/system/src/Tests/Form/FormTest.php b/core/modules/system/src/Tests/Form/FormTest.php index 5a43f49..5132b70 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. @@ -533,7 +533,7 @@ function testDisabledElements() { // All the elements should be marked as disabled, including the ones below // the disabled container. $actual_count = count($disabled_elements); - $expected_count = 41; + $expected_count = 42; $this->assertEqual($actual_count, $expected_count, SafeMarkup::format('Found @actual elements with disabled property (expected @expected).', array( '@actual' => count($disabled_elements), '@expected' => $expected_count, diff --git a/core/modules/system/src/Tests/Form/TriggeringElementProgrammedUnitTest.php b/core/modules/system/src/Tests/Form/TriggeringElementProgrammedUnitTest.php index ae6b82c..f7cffa1 100644 --- a/core/modules/system/src/Tests/Form/TriggeringElementProgrammedUnitTest.php +++ b/core/modules/system/src/Tests/Form/TriggeringElementProgrammedUnitTest.php @@ -19,11 +19,23 @@ */ 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} + */ public function getFormId() { return 'triggering_element_programmed_form'; } diff --git a/core/modules/system/src/Tests/Form/ValidationTest.php b/core/modules/system/src/Tests/Form/ValidationTest.php index 7b6c9e8..e145b72 100644 --- a/core/modules/system/src/Tests/Form/ValidationTest.php +++ b/core/modules/system/src/Tests/Form/ValidationTest.php @@ -8,6 +8,7 @@ namespace Drupal\system\Tests\Form; use Drupal\Core\Render\Element; +use Drupal\Core\Url; use Drupal\simpletest\WebTestBase; /** @@ -206,17 +207,34 @@ 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/fieldset.html.twig b/core/modules/system/templates/fieldset.html.twig index ab6796c..cf4e6fa 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. @@ -22,6 +23,11 @@ */ #} + {% if errors %} +
+ {{ errors }} +
+ {% endif %} {% set legend_span_classes = [ 'fieldset-legend', diff --git a/core/modules/system/templates/form-element.html.twig b/core/modules/system/templates/form-element.html.twig index a961801..f9148c8 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', ] %} {% @@ -61,6 +63,11 @@ ] %} + {% if errors %} +
+ {{ errors }} +
+ {% endif %} {% if label_display in ['before', 'invisible'] %} {{ label }} {% endif %} diff --git a/core/modules/user/src/Tests/UserBlocksTest.php b/core/modules/user/src/Tests/UserBlocksTest.php index 1349264..2a46671 100644 --- a/core/modules/user/src/Tests/UserBlocksTest.php +++ b/core/modules/user/src/Tests/UserBlocksTest.php @@ -43,6 +43,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 ce7bfee..85d9702 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 0000000..64b8705 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Form/FormElementHelperTest.php @@ -0,0 +1,70 @@ +assertSame($expected, FormElementHelper::getElementByName($name, $form)); + } + + /** + * Provides test data. + */ + public function getElementByNameProvider() { + return [ + ['id', [], []], + ['id', ['id' => ['#title' => 'ID']], ['#title' => 'ID']], + ['id', ['fieldset' => ['id' => ['#title' => 'ID']]], ['#title' => 'ID']], + ['fieldset', ['fieldset' => ['id' => ['#title' => 'ID']]], ['id' => ['#title' => 'ID']]], + ]; + } + + /** + * 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() { + return [ + ['id', [], ''], + ['id', ['id' => ['#title' => 'ID']], 'ID'], + ['id', ['fieldset' => ['id' => ['#title' => 'ID']]], 'ID'], + ['fieldset', ['fieldset' => ['id' => ['#title' => 'ID']]], 'ID'], + ]; + } + +} 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 0000000..2769506 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Form/FormErrorHandlerTest.php @@ -0,0 +1,85 @@ +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('this missing element is invalid', 'error'); + $form_error_handler->expects($this->at(1)) + ->method('drupalSetMessage') + ->with('2 errors have been found: Test 1, Test 2', 'error'); + + $form = [ + '#parents' => [], + ]; + $form['test1'] = [ + '#type' => 'textfield', + '#title' => 'Test 1', + '#parents' => ['test1'], + ]; + $form['test2'] = [ + '#type' => 'textfield', + '#title' => 'Test 2', + '#parents' => ['test2'], + ]; + $form_state = new FormState(); + $form_state->setErrorByName('test1', 'invalid'); + $form_state->setErrorByName('test2', 'invalid'); + $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'], + ]; + $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 24c4ddd..b662342 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), + array(NULL, array('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 3ad3d09..84b447c 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php +++ b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php @@ -152,9 +152,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 5165c0c..d701991 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormValidatorTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormValidatorTest.php @@ -20,6 +20,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. * * @covers ::validateForm @@ -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()) @@ -145,38 +171,13 @@ public function testValidateValidFormToken() { } /** - * @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 * * @dataProvider providerTestHandleErrorsWithLimitedValidation */ 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 c15bff2..ac42b8f 100644 --- a/core/tests/Drupal/Tests/UnitTestCase.php +++ b/core/tests/Drupal/Tests/UnitTestCase.php @@ -8,6 +8,7 @@ namespace Drupal\Tests; use Drupal\Component\Utility\Random; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Cache\CacheTagsInvalidatorInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; @@ -203,6 +204,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/classy/templates/form/fieldset.html.twig b/core/themes/classy/templates/form/fieldset.html.twig index f7460cf..aa14b30 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. @@ -20,6 +21,11 @@ */ #} + {% if errors %} +
+ {{ errors }} +
+ {% endif %} {% set legend_span_classes = [ 'fieldset-legend', diff --git a/core/themes/classy/templates/form/form-element.html.twig b/core/themes/classy/templates/form/form-element.html.twig index cf54c20..d238ac5 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 @@ -50,6 +51,7 @@ 'form-item-' ~ name|clean_class, title_display not in ['after', 'before'] ? 'form-no-label', disabled == 'disabled' ? 'form-disabled', + errors ? 'form-error', ] %} {% @@ -59,6 +61,11 @@ ] %} + {% if errors %} +
+ {{ errors }} +
+ {% endif %} {% if label_display in ['before', 'invisible'] %} {{ label }} {% endif %} diff --git a/core/themes/seven/css/components/form.css b/core/themes/seven/css/components/form.css index 72980e5..7deb2e6 100644 --- a/core/themes/seven/css/components/form.css +++ b/core/themes/seven/css/components/form.css @@ -83,6 +83,9 @@ label[for] { height: 7px; } +.form-error-message { + color: #ea2800; +} /* Filter */ .filter-wrapper { @@ -96,6 +99,7 @@ div.description, font-size: 0.9em; } .form-item .description.error { + margin-top: 0; color: #a51b00; } @@ -171,14 +175,6 @@ textarea.form-textarea { width: auto; } -.form-item .password-suggestions { - float: left; /* LTR */ - clear: left; - width: 100%; -} -[dir="rtl"] .form-item .password-suggestions { - float: right; -} .form-item-pass .description { clear: both; } diff --git a/core/modules/system/templates/fieldset.html.twig b/core/themes/seven/templates/fieldset.html.twig similarity index 89% copy from core/modules/system/templates/fieldset.html.twig copy to core/themes/seven/templates/fieldset.html.twig index ab6796c..cf7db0d 100644 --- a/core/modules/system/templates/fieldset.html.twig +++ b/core/themes/seven/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/themes/seven/templates/form-element.html.twig similarity index 94% copy from core/modules/system/templates/form-element.html.twig copy to core/themes/seven/templates/form-element.html.twig index a961801..3a1ef12 100644 --- a/core/modules/system/templates/form-element.html.twig +++ b/core/themes/seven/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 }}