Issue #2190895 by frankcarey, tim.plunkett: Revamp vertical tabs to not save form values

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

@@ -196,6 +196,24 @@ class FormState implements FormStateInterface {
    */
   protected $values = array();
 
+  /**
+   * An associative array of form value keys to be removed by cleanValues().
+   *
+   * Any values that are temporary but must still be displayed as values in
+   * the rendered form should be added to this array using addCleanValueKey().
+   * Initialized with internal Form API values.
+   *
+   * This property is uncacheable.
+   *
+   * @var array
+   */
+  protected $cleanValueKeys = [
+    'form_id',
+    'form_token',
+    'form_build_id',
+    'op',
+  ];
+
   /**
    * The array of values as they were submitted by the user.
    *
@@ -1160,16 +1178,37 @@ class FormState implements FormStateInterface {
     return $this->getBuildInfo()['callback_object'];
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getCleanValueKeys() {
+    return $this->cleanValueKeys;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setCleanValueKeys(array $cleanValueKeys) {
+    $this->cleanValueKeys = $cleanValueKeys;
+    return $this;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function addCleanValueKey($cleanValueKey) {
+    $keys = $this->getCleanValueKeys();
+    $this->setCleanValueKeys(array_merge((array)$keys, [$cleanValueKey]));
+    return $this;
+  }
+
   /**
    * {@inheritdoc}
    */
   public function cleanValues() {
-    // Remove internal Form API values.
-    $this
-      ->unsetValue('form_id')
-      ->unsetValue('form_token')
-      ->unsetValue('form_build_id')
-      ->unsetValue('op');
+    foreach ($this->getCleanValueKeys() as $value) {
+      $this->unsetValue($value);
+    }
 
     // Remove button values.
     // \Drupal::formBuilder()->doBuildForm() collects all button elements in a

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

@@ -1028,15 +1028,43 @@ interface FormStateInterface {
    */
   public function isValidationComplete();
 
+  /**
+   * Gets the keys of the form values that will be cleaned.
+   *
+   * @return array
+   *   An array of form value keys to be cleaned.
+   */
+  public function getCleanValueKeys();
+
+  /**
+   * Sets the keys of the form values that will be cleaned.
+   *
+   * @param array $keys
+   *   An array of form value keys to be cleaned.
+   *
+   * @return $this
+   */
+  public function setCleanValueKeys(array $keys);
+
+  /**
+   * Adds a key to the array of form values that will be cleaned.
+   *
+   * @param string $key
+   *   The form value key to be cleaned.
+   *
+   * @return $this
+   */
+  public function addCleanValueKey($key);
+
   /**
    * Removes internal Form API elements and buttons from submitted form values.
    *
    * This function can be used when a module wants to store all submitted form
    * values, for example, by serializing them into a single database column. In
    * such cases, all internal Form API values and all form button elements
-   * should not be contained, and this function allows to remove them before the
+   * should not be contained, and this function allows their removal before the
    * module proceeds to storage. Next to button elements, the following internal
-   * values are removed:
+   * values are removed by default.
    * - form_id
    * - form_token
    * - form_build_id

core/lib/Drupal/Core/Render/Element/VerticalTabs.php

@@ -100,6 +100,9 @@ class VerticalTabs extends RenderElement {
       '#default_value' => $element['#default_tab'],
       '#attributes' => array('class' => array('vertical-tabs-active-tab')),
     );
+    // Clean up the active tab value so it's not accidentally stored in
+    // settings forms.
+    $form_state->addCleanValueKey($name . '__active_tab');
 
     return $element;
   }

core/modules/system/src/Tests/Form/ElementsVerticalTabsTest.php

@@ -7,7 +7,9 @@
 
 namespace Drupal\system\Tests\Form;
 
+use Drupal\Component\Utility\String;
 use Drupal\simpletest\WebTestBase;
+use Drupal\Component\Serialization\Json;
 
 /**
  * Tests the vertical_tabs form element for expected behavior.
@@ -66,4 +68,12 @@ class ElementsVerticalTabsTest extends WebTestBase {
     $this->drupalGet('form_test/vertical-tabs');
     $this->assertFieldByName('vertical_tabs__active_tab', 'edit-tab3', t('The default vertical tab is correctly selected.'));
   }
+
+  /**
+   * Ensures that vertical tab form values are cleaned.
+   */
+  function testDefaultTabCleaned() {
+    $values = Json::decode($this->drupalPostForm('form_test/form-state-values-clean', [], t('Submit')));
+    $this->assertFalse(isset($values['vertical_tabs__active_tab']), String::format('%element was removed.', ['%element' => 'vertical_tabs__active_tab']));
+  }
 }

core/modules/system/src/Tests/Form/StateValuesCleanTest.php

@@ -8,6 +8,7 @@
 namespace Drupal\system\Tests\Form;
 
 use Drupal\Component\Serialization\Json;
+use Drupal\Component\Utility\String;
 use Drupal\simpletest\WebTestBase;
 
 /**
@@ -49,6 +50,9 @@ class StateValuesCleanTest extends WebTestBase {
     $this->assertFalse(isset($values['baz']['foo']), format_string('%element was removed.', array('%element' => 'foo')));
     $this->assertFalse(isset($values['baz']['baz']), format_string('%element was removed.', array('%element' => 'baz')));
 
+    // Verify values manually added for cleaning were removed.
+    $this->assertFalse(isset($values['wine']), String::format('%element was removed.', ['%element' => 'wine']));
+
     // Verify that nested form value still exists.
     $this->assertTrue(isset($values['baz']['beer']), 'Nested form value still exists.');
 

core/modules/system/tests/modules/form_test/src/Form/FormTestFormStateValuesCleanForm.php

@@ -36,9 +36,26 @@ class FormTestFormStateValuesCleanForm extends FormBase {
     $form['baz']['foo'] = array('#type' => 'button', '#value' => t('Submit'));
     $form['baz']['baz'] = array('#type' => 'submit', '#value' => t('Submit'));
     $form['baz']['beer'] = array('#type' => 'value', '#value' => 2000);
+
+    // Add an arbitrary element and manually set it to be cleaned.
+    // Using $form_state->addCleanValueKey('wine'); didn't work here.
+    $class = get_class($this);
+    $form['wine'] = [
+      '#type' => 'value',
+      '#value' => 3000,
+      '#process' => [[$class, 'cleanValue']],
+    ];
+
     return $form;
   }
 
+  /**
+   * Helper function to clean a value on an element.
+   */
+  public static function cleanValue(&$element, FormStateInterface $form_state, &$complete_form) {
+    $form_state->addCleanValueKey('wine');
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/system/tests/modules/form_test/src/Form/FormTestVerticalTabsForm.php

@@ -51,6 +51,10 @@ class FormTestVerticalTabsForm extends FormBase {
    * {@inheritdoc}
    */
   public function submitForm(array &$form, FormStateInterface $form_state) {
+    $form_state->cleanValues();
+    // This won't have a proper JSON header, but Drupal doesn't check for that
+    // anyway so this is fine until it's replaced with a JsonResponse.
+    print Json::encode($form_state->getValues());
+    exit;
   }
-
 }

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

@@ -530,6 +530,44 @@ class FormStateTest extends UnitTestCase {
     $this->assertTrue($form_state->hasTemporaryValue(array('rainbow_sparkles', 'magic_ponies')), TRUE);
   }
 
+  /**
+   * @covers ::getCleanValueKeys
+   */
+  public function testGetCleanValueKeys() {
+    $form_state = new FormState();
+    $this->assertSame($form_state->getCleanValueKeys(), ['form_id', 'form_token', 'form_build_id', 'op']);
+  }
+
+  /**
+   * @covers ::setCleanValueKeys
+   */
+  public function testSetCleanValueKeys() {
+    $form_state = new FormState();
+    $form_state->setCleanValueKeys(['key1', 'key2']);
+    $this->assertSame($form_state->getCleanValueKeys(), ['key1', 'key2']);
+  }
+
+  /**
+   * @covers ::addCleanValueKey
+   */
+  public function testAddCleanValueKey() {
+    $form_state = new FormState();
+    $form_state->setValue('value_to_clean', 'rainbow_sprinkles');
+    $form_state->addCleanValueKey('value_to_clean');
+    $this->assertSame($form_state->getCleanValueKeys(), ['form_id', 'form_token', 'form_build_id', 'op', 'value_to_clean']);
+    return $form_state;
+  }
+
+  /**
+   * @depends testAddCleanValueKey
+   *
+   * @covers ::cleanValues
+   */
+  public function testCleanValues($form_state) {
+    $form_state->setValue('value_to_keep', 'magic_ponies');
+    $form_state->cleanValues();
+    $this->assertSame($form_state->getValues(), ['value_to_keep' => 'magic_ponies']);
+  }
 }
 
 /**