diff --git a/core/includes/batch.inc b/core/includes/batch.inc index 1675b8c..5eff1dc 100644 --- a/core/includes/batch.inc +++ b/core/includes/batch.inc @@ -379,7 +379,7 @@ function _batch_next_set() { if (isset($current_set['form_submit']) && ($callback = $current_set['form_submit']) && is_callable($callback)) { // We use our stored copies of $form and $form_state to account for // possible alterations by previous form submit handlers. - call_user_func_array($callback, array($batch['form_state']['complete_form'], &$batch['form_state'])); + call_user_func_array($callback, array(&$batch['form_state']['complete_form'], &$batch['form_state'])); } return TRUE; } diff --git a/core/lib/Drupal/Core/Controller/FormController.php b/core/lib/Drupal/Core/Controller/FormController.php index 6ba7ba3..a8c44e5 100644 --- a/core/lib/Drupal/Core/Controller/FormController.php +++ b/core/lib/Drupal/Core/Controller/FormController.php @@ -77,8 +77,7 @@ public function getContentResult(Request $request) { unset($args[0], $args[1]); $form_state['build_info']['args'] = array_values($args); - $form_id = $this->formBuilder->getFormId($form_object, $form_state); - return $this->formBuilder->buildForm($form_id, $form_state); + return $this->formBuilder->buildForm($form_object, $form_state); } /** diff --git a/core/lib/Drupal/Core/Entity/EntityFormBuilder.php b/core/lib/Drupal/Core/Entity/EntityFormBuilder.php index 3823718..c875754 100644 --- a/core/lib/Drupal/Core/Entity/EntityFormBuilder.php +++ b/core/lib/Drupal/Core/Entity/EntityFormBuilder.php @@ -52,7 +52,7 @@ public function getForm(EntityInterface $entity, $operation = 'default', array $ $form_state['build_info']['base_form_id'] = $form_object->getBaseFormID(); $form_state['build_info'] += array('args' => array()); - return $this->formBuilder->buildForm($form_object->getFormId(), $form_state); + return $this->formBuilder->buildForm($form_object, $form_state); } } diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index 32a21af..04b90e5 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -9,6 +9,7 @@ use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\NestedArray; +use Drupal\Component\Utility\String; use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Access\CsrfTokenGenerator; use Drupal\Core\DependencyInjection\ClassResolverInterface; @@ -139,18 +140,16 @@ public function getFormId($form_arg, &$form_state) { $form_arg = $this->classResolver->getInstanceFromDefinition($form_arg); } - // If the $form_arg implements \Drupal\Core\Form\FormInterface, add that as - // the callback object and determine the form ID. - if (is_object($form_arg) && $form_arg instanceof FormInterface) { - $form_state['build_info']['callback_object'] = $form_arg; - if ($form_arg instanceof BaseFormIdInterface) { - $form_state['build_info']['base_form_id'] = $form_arg->getBaseFormID(); - } - return $form_arg->getFormId(); + if (!is_object($form_arg) || !($form_arg instanceof FormInterface)) { + throw new \Exception(String::format('The form argument @form_arg is not a valid form.', array('@form_arg' => $form_arg))); } - // Otherwise, the $form_arg is the form ID. - return $form_arg; + // Add the $form_arg as the callback object and determine the form ID. + $form_state['build_info']['callback_object'] = $form_arg; + if ($form_arg instanceof BaseFormIdInterface) { + $form_state['build_info']['base_form_id'] = $form_arg->getBaseFormID(); + } + return $form_arg->getFormId(); } /** @@ -164,8 +163,7 @@ public function getForm($form_arg) { unset($args[0]); $form_state['build_info']['args'] = array_values($args); - $form_id = $this->getFormId($form_arg, $form_state); - return $this->buildForm($form_id, $form_state); + return $this->buildForm($form_arg, $form_state); } /** @@ -468,15 +466,7 @@ public function retrieveForm($form_id, &$form_state) { // the constructor function itself. $args = $form_state['build_info']['args']; - // If an explicit form builder callback is defined we just use it, otherwise - // we look for a function named after the $form_id. - $callback = $form_id; - if (!empty($form_state['build_info']['callback'])) { - $callback = $form_state['build_info']['callback']; - } - elseif (!empty($form_state['build_info']['callback_object'])) { - $callback = array($form_state['build_info']['callback_object'], 'buildForm'); - } + $callback = array($form_state['build_info']['callback_object'], 'buildForm'); $form = array(); // Assign a default CSS class name based on $form_id. @@ -685,40 +675,8 @@ public function prepareForm($form_id, &$form, &$form_state) { $form += $this->getElementInfo('form'); $form += array('#tree' => FALSE, '#parents' => array()); - - if (!isset($form['#validate'])) { - // Ensure that modules can rely on #validate being set. - $form['#validate'] = array(); - if (isset($form_state['build_info']['callback_object'])) { - $form['#validate'][] = array($form_state['build_info']['callback_object'], 'validateForm'); - } - // Check for a handler specific to $form_id. - elseif (function_exists($form_id . '_validate')) { - $form['#validate'][] = $form_id . '_validate'; - } - // Otherwise check whether this is a shared form and whether there is a - // handler for the shared $form_id. - elseif (isset($form_state['build_info']['base_form_id']) && function_exists($form_state['build_info']['base_form_id'] . '_validate')) { - $form['#validate'][] = $form_state['build_info']['base_form_id'] . '_validate'; - } - } - - if (!isset($form['#submit'])) { - // Ensure that modules can rely on #submit being set. - $form['#submit'] = array(); - if (isset($form_state['build_info']['callback_object'])) { - $form['#submit'][] = array($form_state['build_info']['callback_object'], 'submitForm'); - } - // Check for a handler specific to $form_id. - elseif (function_exists($form_id . '_submit')) { - $form['#submit'][] = $form_id . '_submit'; - } - // Otherwise check whether this is a shared form and whether there is a - // handler for the shared $form_id. - elseif (isset($form_state['build_info']['base_form_id']) && function_exists($form_state['build_info']['base_form_id'] . '_submit')) { - $form['#submit'][] = $form_state['build_info']['base_form_id'] . '_submit'; - } - } + $form['#validate'][] = array($form_state['build_info']['callback_object'], 'validateForm'); + $form['#submit'][] = array($form_state['build_info']['callback_object'], 'submitForm'); // If no #theme has been set, automatically apply theme suggestions. // theme_form() itself is in #theme_wrappers and not #theme. Therefore, the diff --git a/core/modules/field/src/Tests/FieldAttachOtherTest.php b/core/modules/field/src/Tests/FieldAttachOtherTest.php index a058dd6..379d108 100644 --- a/core/modules/field/src/Tests/FieldAttachOtherTest.php +++ b/core/modules/field/src/Tests/FieldAttachOtherTest.php @@ -342,6 +342,7 @@ function testEntityFormDisplayExtractFormValues() { $values_2[1]['value'] = 0; // Pretend the form has been built. + $form_state['build_info']['callback_object'] = \Drupal::entityManager()->getFormObject($entity_type, 'default'); \Drupal::formBuilder()->prepareForm('field_test_entity_form', $form, $form_state); drupal_process_form('field_test_entity_form', $form, $form_state); $form_state['values'][$this->field_name] = $values; diff --git a/core/modules/field_ui/src/Form/FieldEditForm.php b/core/modules/field_ui/src/Form/FieldEditForm.php index aea0a64..9d79cf4 100644 --- a/core/modules/field_ui/src/Form/FieldEditForm.php +++ b/core/modules/field_ui/src/Form/FieldEditForm.php @@ -153,8 +153,6 @@ public function buildForm(array $form, array &$form_state, FieldInstanceConfigIn $form['actions'] = array('#type' => 'actions'); $form['actions']['submit'] = array('#type' => 'submit', '#value' => $this->t('Save field settings')); - $form['#submit'][] = array($this, 'submitForm'); - $form['#validate'][] = array($this, 'validateForm'); return $form; } diff --git a/core/modules/system/src/Tests/Form/ElementsTableSelectTest.php b/core/modules/system/src/Tests/Form/ElementsTableSelectTest.php index 30380ac..b0ce2f8 100644 --- a/core/modules/system/src/Tests/Form/ElementsTableSelectTest.php +++ b/core/modules/system/src/Tests/Form/ElementsTableSelectTest.php @@ -216,6 +216,7 @@ private function formSubmitHelper($form, $edit) { $form_state['input'] = $edit; $form_state['input']['form_id'] = $form_id; + $form_state['build_info']['callback_object'] = new StubForm($form_id, $form); \Drupal::formBuilder()->prepareForm($form_id, $form, $form_state); diff --git a/core/modules/system/src/Tests/Form/FormTest.php b/core/modules/system/src/Tests/Form/FormTest.php index f113bcf..2b4850e 100644 --- a/core/modules/system/src/Tests/Form/FormTest.php +++ b/core/modules/system/src/Tests/Form/FormTest.php @@ -109,6 +109,7 @@ function testRequiredFields() { $form[$element]['#required'] = $required; $form_state['input'][$element] = $empty; $form_state['input']['form_id'] = $form_id; + $form_state['build_info']['callback_object'] = new StubForm($form_id, $form); $form_state['method'] = 'post'; // The form token CSRF protection should not interfere with this test, // so we bypass it by setting the token to FALSE. diff --git a/core/modules/system/src/Tests/Form/StubForm.php b/core/modules/system/src/Tests/Form/StubForm.php new file mode 100644 index 0000000..225371b --- /dev/null +++ b/core/modules/system/src/Tests/Form/StubForm.php @@ -0,0 +1,64 @@ +formId = $form_id; + $this->form = $form; + } + + /** + * {@inheritdoc} + */ + public function getFormId() { + $this->formId; + } + + /** + * {@inheritdoc} + */ + public function buildForm(array $form, array &$form_state) { + return $this->form; + } + + /** + * {@inheritdoc} + */ + public function submitForm(array &$form, array &$form_state) { + } + +} diff --git a/core/modules/views/includes/ajax.inc b/core/modules/views/includes/ajax.inc index 7f6fc89..84c33eb 100644 --- a/core/modules/views/includes/ajax.inc +++ b/core/modules/views/includes/ajax.inc @@ -14,7 +14,7 @@ * some AJAX stuff automatically. * This makes some assumptions about the client. */ -function views_ajax_form_wrapper($form_id, &$form_state) { +function views_ajax_form_wrapper($form_class, &$form_state) { // This won't override settings already in. $form_state += array( 'rerender' => FALSE, @@ -25,7 +25,7 @@ function views_ajax_form_wrapper($form_id, &$form_state) { ), ); - $form = \Drupal::formBuilder()->buildForm($form_id, $form_state); + $form = \Drupal::formBuilder()->buildForm($form_class, $form_state); $output = drupal_render($form); // These forms have the title built in, so set the title here: diff --git a/core/modules/views_ui/src/Form/Ajax/ViewsFormBase.php b/core/modules/views_ui/src/Form/Ajax/ViewsFormBase.php index c6cb25f..ecf8973 100644 --- a/core/modules/views_ui/src/Form/Ajax/ViewsFormBase.php +++ b/core/modules/views_ui/src/Form/Ajax/ViewsFormBase.php @@ -128,7 +128,8 @@ public function getForm(ViewStorageInterface $view, $display_id, $js) { // before rendering the second time. $drupal_add_js_original = _drupal_add_js(); $drupal_add_js = &drupal_static('_drupal_add_js'); - $response = views_ajax_form_wrapper($form_state['form_id'], $form_state); + $form_class = get_class($form_state['build_info']['callback_object']); + $response = views_ajax_form_wrapper($form_class, $form_state); // If the form has not been submitted, or was not set for rerendering, stop. if (!$form_state['submitted'] || !empty($form_state['rerender'])) { @@ -151,7 +152,7 @@ public function getForm(ViewStorageInterface $view, $display_id, $js) { return new RedirectResponse(url($form_path, array('absolute' => TRUE))); } $form_state['path'] = $form_path; - $response = views_ajax_form_wrapper($form_state['form_id'], $form_state); + $response = views_ajax_form_wrapper($form_class, $form_state); } elseif (!$form_state['ajax']) { // if nothing on the stack, non-js forms just go back to the main view editor. diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityFormBuilderTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityFormBuilderTest.php index 0242a44..14622d2 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityFormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityFormBuilderTest.php @@ -49,11 +49,6 @@ protected function setUp() { * @covers ::getForm() */ public function testGetForm() { - $this->formBuilder->expects($this->once()) - ->method('buildForm') - ->with('the_form_id', $this->isType('array')) - ->will($this->returnValue('the form contents')); - $form_controller = $this->getMock('Drupal\Core\Entity\EntityFormInterface'); $form_controller->expects($this->any()) ->method('getFormId') @@ -63,6 +58,11 @@ public function testGetForm() { ->with('the_entity_type', 'default') ->will($this->returnValue($form_controller)); + $this->formBuilder->expects($this->once()) + ->method('buildForm') + ->with($form_controller, $this->isType('array')) + ->will($this->returnValue('the form contents')); + $entity = $this->getMock('Drupal\Core\Entity\EntityInterface'); $entity->expects($this->once()) ->method('getEntityTypeId') diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index fbef067..3f4e0ab 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -19,6 +19,8 @@ class FormBuilderTest extends FormTestBase { /** * Tests the getFormId() method with a string based form ID. + * + * @expectedException \Exception */ public function testGetFormIdWithString() { $form_arg = 'foo'; @@ -121,8 +123,6 @@ public function testHandleFormStateResponse($class, $form_state_key) { })); $form_state = array(); - $this->formBuilder->getFormId($form_arg, $form_state); - try { $form_state['values'] = array(); $form_state['input']['form_id'] = $form_id; @@ -177,8 +177,6 @@ public function testHandleRedirectWithResponse() { })); $form_state = array(); - $this->formBuilder->getFormId($form_arg, $form_state); - try { $form_state['values'] = array(); $form_state['input']['form_id'] = $form_id; @@ -190,8 +188,11 @@ public function testHandleRedirectWithResponse() { } $this->assertSame($response, $form_state['response']); } + /** * Tests the getForm() method with a string based form ID. + * + * @expectedException \Exception */ public function testGetFormWithString() { $form_id = 'test_form_id'; @@ -233,6 +234,8 @@ public function testGetFormWithClassString() { /** * Tests the buildForm() method with a string based form ID. + * + * @expectedException \Exception */ public function testBuildFormWithString() { $form_id = 'test_form_id'; @@ -318,7 +321,13 @@ public function testGetCache() { // FormBuilder::buildForm() will be called twice, but the form object will // only be called once due to caching. - $form_arg = $this->getMockForm($form_id, $expected_form, 1); + $form_arg = $this->getMock('Drupal\Core\Form\FormInterface'); + $form_arg->expects($this->exactly(2)) + ->method('getFormId') + ->will($this->returnValue($form_id)); + $form_arg->expects($this->once()) + ->method('buildForm') + ->will($this->returnValue($expected_form)); // The CSRF token is checked each time. $this->csrfToken->expects($this->exactly(2)) @@ -359,7 +368,7 @@ public function testGetCache() { // use the form cache. $form_state['input']['form_id'] = $form_id; $form_state['input']['form_build_id'] = $form['#build_id']; - $this->formBuilder->buildForm($form_id, $form_state); + $this->formBuilder->buildForm($form_arg, $form_state); $this->assertEmpty($form_state['errors']); } @@ -395,6 +404,9 @@ public function testUniqueHtmlId() { // Mock a form object that will be built two times. $form_arg = $this->getMock('Drupal\Core\Form\FormInterface'); $form_arg->expects($this->exactly(2)) + ->method('getFormId') + ->will($this->returnValue($form_id)); + $form_arg->expects($this->exactly(2)) ->method('buildForm') ->will($this->returnValue($expected_form)); diff --git a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php index a6c2e76..9bb00d2 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php +++ b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php @@ -237,7 +237,7 @@ protected function simulateFormSubmission($form_id, FormInterface $form_arg, arr $form_state['input']['op'] = 'Submit'; $form_state['programmed'] = $programmed; $form_state['submitted'] = TRUE; - return $this->formBuilder->buildForm($form_id, $form_state); + return $this->formBuilder->buildForm($form_arg, $form_state); } /**