diff --git a/core/lib/Drupal/Core/Render/Element/MachineName.php b/core/lib/Drupal/Core/Render/Element/MachineName.php index c1b1fa05c2..6e3ccb6b32 100644 --- a/core/lib/Drupal/Core/Render/Element/MachineName.php +++ b/core/lib/Drupal/Core/Render/Element/MachineName.php @@ -213,8 +213,7 @@ public static function processMachineName(&$element, FormStateInterface $form_st /** * Form element validation handler for machine_name elements. * - * Note that #maxlength is validated by the form validator already, in - * \Drupal\Core\Form\FormValidatorInterface::validateForm(). + * Note that #maxlength is validated by _form_validate() already. * * This checks that the submitted value: * - Does not contain the replacement character only. @@ -250,10 +249,12 @@ public static function validateMachineName(&$element, FormStateInterface $form_s // Verify that the machine name is unique. Check form state if we need to // explicitly validate the element. This happens every time when an error // has been set, since this breaks on AJAX requests - if ($form_state->get('needs_revalidation') === $element['#name'] || $element['#default_value'] !== $element['#value']) { + $needs_revalidating = $form_state->get('machine_name.needs_revalidating') ?: []; + if ($element['#default_value'] !== $element['#value'] || isset($needs_revalidating[$element['#name']])) { $function = $element['#machine_name']['exists']; if (call_user_func($function, $element['#value'], $element, $form_state)) { - $form_state->set('needs_revalidation', $element['#name']); + $needs_revalidating[$element['#name']] = TRUE; + $form_state->set('machine_name.needs_revalidating', $needs_revalidating); $form_state->setError($element, t('The machine-readable name is already in use. It must be unique.')); } } diff --git a/core/modules/media/src/MediaTypeForm.php b/core/modules/media/src/MediaTypeForm.php index cd01c13a1c..d696556016 100644 --- a/core/modules/media/src/MediaTypeForm.php +++ b/core/modules/media/src/MediaTypeForm.php @@ -229,18 +229,6 @@ public function form(array $form, FormStateInterface $form_state) { return $form; } - /** - * Form submission handler to rebuild the form on select submit. - * - * @param array $form - * Full form array. - * @param \Drupal\Core\Form\FormStateInterface $form_state - * Current form state. - */ - public static function rebuildSubmit(array &$form, FormStateInterface $form_state) { - $form_state->setRebuild(); - } - /** * Prepares workflow options to be used in the 'checkboxes' form element. * diff --git a/core/modules/media/tests/src/FunctionalJavascript/MediaTypeCreationTest.php b/core/modules/media/tests/src/FunctionalJavascript/MediaTypeCreationTest.php index 2844410ed8..207b542f95 100644 --- a/core/modules/media/tests/src/FunctionalJavascript/MediaTypeCreationTest.php +++ b/core/modules/media/tests/src/FunctionalJavascript/MediaTypeCreationTest.php @@ -99,9 +99,7 @@ public function testMediaTypeCreationReuseSourceField() { // Create a new media type, which should create a new field we can reuse. $this->drupalGet('/admin/structure/media/add'); - // Choose the source plugin _before_ setting the label and machine name in - // order to guard against the regression in - // https://www.drupal.org/project/drupal/issues/2557299. + // Choose the source plugin before setting the label and machine name. $page->selectFieldOption('Media source', 'test'); $result = $assert_session->waitForElementVisible('css', 'fieldset[data-drupal-selector="edit-source-configuration"]'); $this->assertNotEmpty($result); @@ -115,8 +113,7 @@ public function testMediaTypeCreationReuseSourceField() { $this->drupalGet('admin/structure/media/add'); // Select the media source used by our media type. Do this before setting - // the label and machine name in order to guard against the regression in - // https://www.drupal.org/project/drupal/issues/2557299. + // the label and machine name. $assert_session->fieldExists('Media source'); $assert_session->optionExists('Media source', 'test'); $page->selectFieldOption('Media source', 'test'); diff --git a/core/modules/system/tests/modules/form_test/src/Form/FormTestMachineNameValidationForm.php b/core/modules/system/tests/modules/form_test/src/Form/FormTestMachineNameValidationForm.php index 04ecc0a521..ff2dcaac58 100644 --- a/core/modules/system/tests/modules/form_test/src/Form/FormTestMachineNameValidationForm.php +++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestMachineNameValidationForm.php @@ -21,12 +21,16 @@ public function getFormId() { * {@inheritdoc} */ public function buildForm(array $form, FormStateInterface $form_state) { + // Disable client-side validation so that we can test AJAX requests with + // invalid input. + $form['#attributes']['novalidate'] = 'novalidate'; + $form['name'] = [ '#type' => 'textfield', '#default_value' => $form_state->getValue('name'), '#maxlength' => 50, '#required' => TRUE, - '#title' => 'Name' + '#title' => 'Name', ]; // The default value simulates how an entity form works, which has default @@ -47,6 +51,20 @@ public function buildForm(array $form, FormStateInterface $form_state) { ], ]; + // Test support for multiple machine names on the form. Although this has + // the default value duplicate it should not generate an error because it + // is the default value. + $form['id2'] = [ + '#type' => 'machine_name', + '#default_value' => 'duplicate', + '#maxlength' => 50, + '#required' => TRUE, + '#machine_name' => [ + 'exists' => [$this, 'load'], + 'source' => ['name'], + ], + ]; + $form['snack'] = [ '#type' => 'select', '#title' => $this->t('Select a snack'), @@ -57,7 +75,6 @@ public function buildForm(array $form, FormStateInterface $form_state) { ], '#required' => TRUE, '#ajax' => [ - 'trigger_as' => ['name' => 'snack_configure'], 'callback' => '::buildAjaxSnackConfigureForm', 'wrapper' => 'snack-config-form', 'method' => 'replace', @@ -71,29 +88,6 @@ public function buildForm(array $form, FormStateInterface $form_state) { ], '#tree' => TRUE, ]; - $form['snack_configure_button'] = [ - '#type' => 'submit', - '#name' => 'snack_configure', - '#value' => 'Configure snack', - '#limit_validation_errors' => [['snack']], - '#validate' => ['::buildAjaxSnackConfigureFormValidate'], - '#submit' => ['::buildAjaxSnackConfigureFormSubmit'], - '#executes_submit_callback' => TRUE, - '#ajax' => [ - 'callback' => '::buildAjaxSnackConfigureForm', - 'wrapper' => 'snack-config-form', - ], - '#attributes' => ['class' => ['js-hide']], - ]; - $form['snack_configs']['apple']['#type'] = 'details'; - $form['snack_configs']['apple']['#title'] = 'Configure Apple'; - $form['snack_configs']['apple']['#open'] = TRUE; - $form['snack_configs']['pear']['#type'] = 'details'; - $form['snack_configs']['pear']['#title'] = 'Configure pear'; - $form['snack_configs']['pear']['#open'] = TRUE; - $form['snack_configs']['potato']['#type'] = 'details'; - $form['snack_configs']['potato']['#title'] = 'Configure potato'; - $form['snack_configs']['potato']['#open'] = TRUE; $form['submit'] = [ '#type' => 'submit', @@ -130,7 +124,7 @@ public function buildAjaxSnackConfigureForm(array $form, FormStateInterface $for * @return bool */ public function load($machine_name) { - if ($machine_name === 'duplicate') { + if (strpos($machine_name, 'duplicate') !== FALSE) { return TRUE; } diff --git a/core/modules/system/tests/src/Functional/Form/FormTest.php b/core/modules/system/tests/src/Functional/Form/FormTest.php index 1d9c6399d5..2407a3f51a 100644 --- a/core/modules/system/tests/src/Functional/Form/FormTest.php +++ b/core/modules/system/tests/src/Functional/Form/FormTest.php @@ -770,36 +770,4 @@ public function testRequiredAttribute() { $this->assertTrue(!empty($element), 'The textarea has the proper required attribute.'); } - /** - * Tests that a machine name is still required after an AJAX submit. - * - * This protects against the regression in https://www.drupal.org/node/2557299. - */ - public function testMachineNameRequiredFormAjaxSubmit() { - $this->drupalGet('/form-test/form-test-machine-name-validation'); - - $edit = []; - $this->submitForm($edit, 'Save'); - $this->assertResponse(200); - $this->assertText('Machine-readable name field is required.'); - - $edit = [ - 'name' => 'test 1', - 'id' => 'machine1', - 'snack' => 'apple' - ]; - $this->submitForm($edit, 'Save'); - $this->assertResponse(200); - $this->assertText('The form_test_machine_name_validation_form form has been submitted successfully.'); - - $edit = [ - 'name' => 'test 2', - 'id' => 'duplicate', - 'snack' => 'potato', - ]; - $this->submitForm($edit, 'snack_configure'); - $this->submitForm($edit, 'Save'); - $this->assertResponse(200); - $this->assertText('The machine-readable name is already in use. It must be unique.'); - } } diff --git a/core/modules/system/tests/src/FunctionalJavascript/MachineNameValidationTest.php b/core/modules/system/tests/src/FunctionalJavascript/MachineNameValidationTest.php new file mode 100644 index 0000000000..e09ca6cf4c --- /dev/null +++ b/core/modules/system/tests/src/FunctionalJavascript/MachineNameValidationTest.php @@ -0,0 +1,57 @@ +assertSession(); + $this->drupalGet('/form-test/form-test-machine-name-validation'); + + // Test errors after with no AJAX. + $assert->buttonExists('Save')->press(); + $assert->pageTextContains('Machine-readable name field is required.'); + // Ensure only the first machine name field has an error. + $this->assertTrue($assert->fieldExists('id')->hasClass('error')); + $this->assertFalse($assert->fieldExists('id2')->hasClass('error')); + + // Test a successful submit after using AJAX. + $assert->fieldExists('Name')->setValue('test 1'); + $assert->fieldExists('id')->setValue('test_1'); + $assert->selectExists('snack')->selectOption('apple'); + $assert->assertWaitOnAjaxRequest(); + $assert->buttonExists('Save')->press(); + $assert->pageTextContains('The form_test_machine_name_validation_form form has been submitted successfully.'); + + // Test errors after using AJAX. + $assert->fieldExists('Name')->setValue('duplicate'); + $this->assertJsCondition('document.forms[0].id.value === "duplicate"'); + $assert->fieldExists('id2')->setValue('duplicate2'); + $assert->selectExists('snack')->selectOption('potato'); + $assert->assertWaitOnAjaxRequest(); + $assert->buttonExists('Save')->press(); + $assert->pageTextContains('The machine-readable name is already in use. It must be unique.'); + // Ensure both machine name fields both have errors. + $this->assertTrue($assert->fieldExists('id')->hasClass('error')); + $this->assertTrue($assert->fieldExists('id2')->hasClass('error')); + + } + +}