Issue #1831560 by acbramley, CRZDEV, mvwensen, fago, David_Rothstein,...

Issue #1831560 by acbramley, CRZDEV, mvwensen, fago, David_Rothstein, harsha012, Lendude, mrinalini9, Devin Carlson, borisson_, Gábor Hojtsy: Remove Html::resetSeenIds() call during form processing

core/lib/Drupal/Core/Form/FormBuilder.php

@@ -385,6 +385,17 @@ public function rebuildForm($form_id, FormStateInterface &$form_state, $old_form
       $form_state->setCached();
     }
 
+    // \Drupal\Component\Utility\Html::getUniqueId() maintains a cache of
+    // element IDs it has seen, so it can prevent duplicates. We want to be
+    // sure we reset that cache when a form is processed, so scenarios that
+    // result in the form being built behind the scenes and again for the
+    // browser don't increment all the element IDs needlessly.
+    if (!FormState::hasAnyErrors()) {
+      // We only reset HTML ID's when there are no validation errors as this can
+      // cause ID collisions with other forms on the page otherwise.
+      Html::resetSeenIds();
+    }
+
     // If only parts of the form will be returned to the browser (e.g., Ajax or
     // RIA clients), or if the form already had a new build ID regenerated when
     // it was retrieved from the form cache, reuse the existing #build_id.
@@ -576,16 +587,6 @@ public function processForm($form_id, &$form, FormStateInterface &$form_state) {
       }
       $this->formValidator->validateForm($form_id, $form, $form_state);
 
-      // \Drupal\Component\Utility\Html::getUniqueId() maintains a cache of
-      // element IDs it has seen, so it can prevent duplicates. We want to be
-      // sure we reset that cache when a form is processed, so scenarios that
-      // result in the form being built behind the scenes and again for the
-      // browser don't increment all the element IDs needlessly.
-      if (!FormState::hasAnyErrors()) {
-        // In case of errors, do not break HTML IDs of other forms.
-        Html::resetSeenIds();
-      }
-
       // If there are no errors and the form is not rebuilding, submit the form.
       if (!$form_state->isRebuilding() && !FormState::hasAnyErrors()) {
         $submit_response = $this->formSubmitter->doSubmitForm($form, $form_state);

core/modules/views/tests/src/Functional/Plugin/ExposedFormTest.php

@@ -245,11 +245,11 @@ public function testExposedBlock() {
 
     // Test that the correct option is selected after form submission.
     $this->assertCacheContext('url');
-    $this->assertOptionSelected('edit-type', 'All');
+    $this->assertOptionSelected('Content: Type', 'All');
     foreach (['All', 'article', 'page'] as $argument) {
       $this->drupalGet('test_exposed_block', ['query' => ['type' => $argument]]);
       $this->assertCacheContext('url');
-      $this->assertOptionSelected('edit-type', $argument);
+      $this->assertOptionSelected('Content: Type', $argument);
     }
   }
 

core/modules/views/tests/src/Kernel/Plugin/ExposedFormRenderTest.php

@@ -49,7 +49,7 @@ public function testExposedFormRender() {
     $expected_action = $view->display_handler->getUrlInfo()->toString();
     $this->assertFieldByXPath('//form/@action', $expected_action, 'The expected value for the action attribute was found.');
     // Make sure the description is shown.
-    $result = $this->xpath('//form//div[contains(@id, :id) and normalize-space(text())=:description]', [':id' => 'edit-type--description', ':description' => t('Exposed description')]);
+    $result = $this->xpath('//form//div[contains(@id, :id) and normalize-space(text())=:description]', [':id' => 'edit-type--2--description', ':description' => t('Exposed description')]);
     $this->assertCount(1, $result, 'Filter description was found.');
   }
 

core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php

@@ -485,6 +485,36 @@ public function testUniqueHtmlId() {
     $this->assertSame('test-form-id--2', $form['#id']);
   }
 
+  /**
+   * Tests that HTML IDs are unique between 2 forms with the same element names.
+   */
+  public function testUniqueElementHtmlId() {
+    $form_id_1 = 'test_form_id';
+    $form_id_2 = 'test_form_id_2';
+    $expected_form = $form_id_1();
+
+    // Mock 2 form objects that will be built once each.
+    $form_arg_1 = $this->createMock('Drupal\Core\Form\FormInterface');
+    $form_arg_1->expects($this->exactly(1))
+      ->method('getFormId')
+      ->will($this->returnValue($form_id_1));
+    $form_arg_1->expects($this->exactly(1))
+      ->method('buildForm')
+      ->will($this->returnValue($expected_form));
+    $form_arg_2 = $this->createMock('Drupal\Core\Form\FormInterface');
+    $form_arg_2->expects($this->exactly(1))
+      ->method('getFormId')
+      ->will($this->returnValue($form_id_2));
+    $form_arg_2->expects($this->exactly(1))
+      ->method('buildForm')
+      ->will($this->returnValue($expected_form));
+    $form_state = new FormState();
+    $form_1 = $this->simulateFormSubmission($form_id_1, $form_arg_1, $form_state);
+    $form_state = new FormState();
+    $form_2 = $this->simulateFormSubmission($form_id_2, $form_arg_2, $form_state);
+    $this->assertNotSame($form_1['actions']["#id"], $form_2['actions']["#id"]);
+  }
+
   /**
    * Tests that a cached form is deleted after submit.
    */