diff --git a/core/lib/Drupal/Core/Form/FormErrorHandler.php b/core/lib/Drupal/Core/Form/FormErrorHandler.php index a4e3605..df736b0 100644 --- a/core/lib/Drupal/Core/Form/FormErrorHandler.php +++ b/core/lib/Drupal/Core/Form/FormErrorHandler.php @@ -49,9 +49,6 @@ public function handleFormErrors(array &$form, FormStateInterface $form_state) { $this->setElementErrorsFromFormState($form, $form_state); } - // Reset the list of elements with errors. - $this->errorLinkElements = []; - return $this; } @@ -66,7 +63,6 @@ public function handleFormErrors(array &$form, FormStateInterface $form_state) { 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); @@ -74,41 +70,33 @@ protected function displayErrorMessages(array $form, FormStateInterface $form_st // Only show links to erroneous elements that are visible. $is_visible_element = Element::isVisibleElement($form_element); - // And don't show links to elements with suppressed messages. - // Most often their parent element is used for inline errors. - $show_message = empty($form_element['#error_no_message']); - // And only show links for elements that have a title themselves or have + // Only show links for elements that have a title themselves or have // children with a title. $has_title = !empty($title); - // And only show links for elements with an ID. + // Only show links for elements with an ID. $has_id = !empty($form_element['#id']); - if ($is_visible_element && $show_message && $has_title && $has_id) { - $error_links[] = $this->l($title, Url::fromRoute('', [], ['fragment' => $form_element['#id'], 'external' => TRUE])); + // 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]); } - // In any case hide any messages witch are suppressed. - elseif (!$show_message) { + 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]); } } - // For all left over errors set normal error messages. + // Set normal error messages for all remaining errors. foreach ($errors as $error) { $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), + '!errors' => SafeMarkup::set(implode(', ', $error_links)), ]); $this->drupalSetMessage($message, 'error'); } diff --git a/core/tests/Drupal/Tests/Core/Form/FormElementHelperTest.php b/core/tests/Drupal/Tests/Core/Form/FormElementHelperTest.php index 64b8705..31f4341 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormElementHelperTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormElementHelperTest.php @@ -35,12 +35,74 @@ public function testGetElementByName($name, $form, $expected) { * 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']]], + $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; } /** @@ -59,12 +121,59 @@ public function testGetElementTitle($name, $form, $expected) { * 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'], + $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 index 88af835..53d7e76 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormErrorHandlerTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormErrorHandlerTest.php @@ -32,10 +32,16 @@ public function testDisplayErrorMessages() { $form_error_handler->expects($this->at(0)) ->method('drupalSetMessage') - ->with('this missing element is invalid', 'error'); + ->with('no title given', 'error'); $form_error_handler->expects($this->at(1)) ->method('drupalSetMessage') - ->with('3 errors have been found: Test 1, Test 2, Test 3', 'error'); + ->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' => [], @@ -48,7 +54,7 @@ public function testDisplayErrorMessages() { ]; $form['test2'] = [ '#type' => 'textfield', - '#title' => 'Test 2', + '#title' => 'Test 2 & a half', '#parents' => ['test2'], '#id' => 'edit-test2', ]; @@ -61,10 +67,31 @@ public function testDisplayErrorMessages() { '#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']);