From b2da8c27986e12cd9d94a0fc5ae957bb8526aeec Mon Sep 17 00:00:00 2001 From: Tim Greller Date: Tue, 21 Oct 2025 15:50:56 +0000 Subject: [PATCH 1/3] fix: support back button when multiple forms used on the same page --- src/EventListener/CompileFormFieldsListener.php | 12 ++++++++++-- src/EventListener/PrepareFomDataListener.php | 6 +++++- src/FormManager.php | 7 +++++-- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/EventListener/CompileFormFieldsListener.php b/src/EventListener/CompileFormFieldsListener.php index e494d8d..e0c63ad 100644 --- a/src/EventListener/CompileFormFieldsListener.php +++ b/src/EventListener/CompileFormFieldsListener.php @@ -77,8 +77,16 @@ public function __invoke(array $formFields, string $formId, Form $form): array $manager->storeStepData($stepData); } - // Redirect back if asked for it - if ('back' === $request->request->get('mp_form_pageswitch')) { + // Redirect back if asked for it. Support a per-form pageswitch name (e.g. + // "mp_form_pageswitch_123") so forms on the same page don't interfere with + // each other. Fall back to the legacy "mp_form_pageswitch" for BC. + $pageswitchKey = 'mp_form_pageswitch_'.$form->id; + + $pageSwitchValue = $request->request->has($pageswitchKey) + ? $request->request->get($pageswitchKey) + : $request->request->get('mp_form_pageswitch'); + + if ('back' === $pageSwitchValue) { $manager->redirectToStep($manager->getPreviousStep()); } diff --git a/src/EventListener/PrepareFomDataListener.php b/src/EventListener/PrepareFomDataListener.php index 8260c7d..5507292 100644 --- a/src/EventListener/PrepareFomDataListener.php +++ b/src/EventListener/PrepareFomDataListener.php @@ -39,7 +39,11 @@ public function __invoke(array &$submitted, array &$labels, array $fields, Form $submittedBag = new ParameterBag($submitted); $labelsBag = new ParameterBag($labels); - $pageSwitchValue = $submittedBag->get('mp_form_pageswitch', ''); + // Support per-form pageswitch names (e.g. "mp_form_pageswitch_123") to + // allow multiple forms on one page. Fall back to legacy "mp_form_pageswitch". + $perFormKey = 'mp_form_pageswitch_'.$form->id; + + $pageSwitchValue = $submittedBag->get($perFormKey, $submittedBag->get('mp_form_pageswitch', '')); // Store data in session $stepData = $manager->getDataOfCurrentStep(); diff --git a/src/FormManager.php b/src/FormManager.php index a3185e9..ed7d6b4 100644 --- a/src/FormManager.php +++ b/src/FormManager.php @@ -422,8 +422,11 @@ private function prepare(): void if ($this->isPageBreak($formField)) { $isPageBreakLastFormField = true; - // Set the name on the model, otherwise one has to enter it in the back end every time - $formField->name = $formField->type; + // Set a unique name on the model (per form) to allow multiple forms on the + // same page to work independently. Previously this was always + // "mp_form_pageswitch" which caused collisions when multiple forms were + // present. + $formField->name = $formField->type.'_'.$this->formModel->id; // Increase counter ++$i; From 9aa4ee57a6632429f76ca9fd44944c0c4f35f1df Mon Sep 17 00:00:00 2001 From: Tim Greller Date: Tue, 21 Oct 2025 16:14:04 +0000 Subject: [PATCH 2/3] Filter out the mp_form_pageswitch field, even when it has an id appended --- src/EventListener/PrepareFomDataListener.php | 1 + src/Widget/Placeholder.php | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/EventListener/PrepareFomDataListener.php b/src/EventListener/PrepareFomDataListener.php index 5507292..2482769 100644 --- a/src/EventListener/PrepareFomDataListener.php +++ b/src/EventListener/PrepareFomDataListener.php @@ -69,6 +69,7 @@ public function __invoke(array &$submitted, array &$labels, array $fields, Form // explanation fields) as the step data would be empty and getFirstInvalidStep() // would return the one before the empty step. unset($submitted['mp_form_pageswitch']); + unset($submitted[$perFormKey]); // Add session data for Contao 4.13 if (version_compare(ContaoCoreBundle::getVersion(), '5.0', '<')) { diff --git a/src/Widget/Placeholder.php b/src/Widget/Placeholder.php index 01e9988..812eb91 100644 --- a/src/Widget/Placeholder.php +++ b/src/Widget/Placeholder.php @@ -143,7 +143,8 @@ private function generateTokens(): array } // Also skip Contao internal tokens and the page switch element - if (\in_array($k, ['REQUEST_TOKEN', 'FORM_SUBMIT', 'mp_form_pageswitch'], true)) { + if (\in_array($k, ['REQUEST_TOKEN', 'FORM_SUBMIT', 'mp_form_pageswitch'], true) + || str_starts_with($k, 'mp_form_pageswitch_')) { continue; } From a9ce21724ab825bfd51dec7239d4d93f32041ef3 Mon Sep 17 00:00:00 2001 From: Tim Greller Date: Tue, 28 Oct 2025 23:34:56 +0100 Subject: [PATCH 3/3] Extract the pageswitch key definition to a method in FormManager --- src/EventListener/CompileFormFieldsListener.php | 13 +++---------- src/EventListener/PrepareFomDataListener.php | 11 ++++------- src/FormManager.php | 10 ++++++++++ src/Widget/Placeholder.php | 3 +-- .../EventListener/PrepareFormDataListenerTest.php | 14 +++++++------- 5 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/EventListener/CompileFormFieldsListener.php b/src/EventListener/CompileFormFieldsListener.php index e0c63ad..47e5803 100644 --- a/src/EventListener/CompileFormFieldsListener.php +++ b/src/EventListener/CompileFormFieldsListener.php @@ -77,16 +77,9 @@ public function __invoke(array $formFields, string $formId, Form $form): array $manager->storeStepData($stepData); } - // Redirect back if asked for it. Support a per-form pageswitch name (e.g. - // "mp_form_pageswitch_123") so forms on the same page don't interfere with - // each other. Fall back to the legacy "mp_form_pageswitch" for BC. - $pageswitchKey = 'mp_form_pageswitch_'.$form->id; - - $pageSwitchValue = $request->request->has($pageswitchKey) - ? $request->request->get($pageswitchKey) - : $request->request->get('mp_form_pageswitch'); - - if ('back' === $pageSwitchValue) { + // Redirect back if asked for it, using the per-form page switch name so + // forms on the same page don't interfere with each other. + if ('back' === $request->request->get($manager->getPageSwitchFormFieldName())) { $manager->redirectToStep($manager->getPreviousStep()); } diff --git a/src/EventListener/PrepareFomDataListener.php b/src/EventListener/PrepareFomDataListener.php index 2482769..5109f5b 100644 --- a/src/EventListener/PrepareFomDataListener.php +++ b/src/EventListener/PrepareFomDataListener.php @@ -39,11 +39,9 @@ public function __invoke(array &$submitted, array &$labels, array $fields, Form $submittedBag = new ParameterBag($submitted); $labelsBag = new ParameterBag($labels); - // Support per-form pageswitch names (e.g. "mp_form_pageswitch_123") to - // allow multiple forms on one page. Fall back to legacy "mp_form_pageswitch". - $perFormKey = 'mp_form_pageswitch_'.$form->id; - - $pageSwitchValue = $submittedBag->get($perFormKey, $submittedBag->get('mp_form_pageswitch', '')); + // Use the per-form page switch name so multiple forms on the same page don't interfere. + $pageSwitchKey = $manager->getPageSwitchFormFieldName(); + $pageSwitchValue = $submittedBag->get($pageSwitchKey, ''); // Store data in session $stepData = $manager->getDataOfCurrentStep(); @@ -68,8 +66,7 @@ public function __invoke(array &$submitted, array &$labels, array $fields, Form // need it because otherwise it's not possible to have an empty step (e.g. only // explanation fields) as the step data would be empty and getFirstInvalidStep() // would return the one before the empty step. - unset($submitted['mp_form_pageswitch']); - unset($submitted[$perFormKey]); + unset($submitted[$pageSwitchKey]); // Add session data for Contao 4.13 if (version_compare(ContaoCoreBundle::getVersion(), '5.0', '<')) { diff --git a/src/FormManager.php b/src/FormManager.php index ed7d6b4..b7a9bfa 100644 --- a/src/FormManager.php +++ b/src/FormManager.php @@ -347,6 +347,16 @@ private function isPageBreak(FormFieldModel $formField): bool return 'mp_form_pageswitch' === $formField->type; } + /** + * Returns the form field name used for the page switch control for this form. + */ + public function getPageSwitchFormFieldName(): string + { + $this->prepare(); + + return 'mp_form_pageswitch_'.$this->formModel->id; + } + private function loadFormFieldModels(): void { $collection = $this->contaoFramework->getAdapter(FormFieldModel::class)->findPublishedByPid($this->formModel->id); diff --git a/src/Widget/Placeholder.php b/src/Widget/Placeholder.php index 812eb91..5e9192a 100644 --- a/src/Widget/Placeholder.php +++ b/src/Widget/Placeholder.php @@ -143,8 +143,7 @@ private function generateTokens(): array } // Also skip Contao internal tokens and the page switch element - if (\in_array($k, ['REQUEST_TOKEN', 'FORM_SUBMIT', 'mp_form_pageswitch'], true) - || str_starts_with($k, 'mp_form_pageswitch_')) { + if (\in_array($k, ['REQUEST_TOKEN', 'FORM_SUBMIT', $manager->getPageSwitchFormFieldName()], true)) { continue; } diff --git a/tests/EventListener/PrepareFormDataListenerTest.php b/tests/EventListener/PrepareFormDataListenerTest.php index 324ef8b..f76da8e 100644 --- a/tests/EventListener/PrepareFormDataListenerTest.php +++ b/tests/EventListener/PrepareFormDataListenerTest.php @@ -36,7 +36,7 @@ public function testDataIsStoredProperlyAndDoesNotAdjustHookParametersIfNotOnLas $listener = new PrepareFomDataListener($factory, $this->createMock(RequestStack::class), $this->createMock(FileUploadNormalizer::class)); - $submitted = ['submitted2' => 'foobar', 'mp_form_pageswitch' => 'continue']; + $submitted = ['submitted2' => 'foobar', 'mp_form_pageswitch_42' => 'continue']; $labels = []; $fields = []; $files = []; @@ -45,27 +45,27 @@ public function testDataIsStoredProperlyAndDoesNotAdjustHookParametersIfNotOnLas $listener($submitted, $labels, $fields, $form, $files); } catch (RedirectResponseException $exception) { $this->assertSame( - 'https://www.example.com/form?step=2&ref='.self::SESSION_IDENTIFIER, + 'https://www.example.com/form?step=2&ref=' . self::SESSION_IDENTIFIER, $exception->getResponse()->headers->get('Location'), ); } - $this->assertSame(['submitted2' => 'foobar', 'mp_form_pageswitch' => 'continue'], $submitted); // "mp_form_pageswitch" should not be removed + $this->assertSame(['submitted2' => 'foobar', 'mp_form_pageswitch_42' => 'continue'], $submitted); // pageswitch should not be removed $this->assertSame([], $labels); // Test we do not modify the hook parameters if not in last step $manager = $factory->forFormId(42); - $this->assertSame(['submitted1' => 'foobar', 'submitted2' => 'foobar', 'mp_form_pageswitch' => 'continue'], $manager->getDataOfAllSteps()->getAllSubmitted()); + $this->assertSame(['submitted1' => 'foobar', 'submitted2' => 'foobar', 'mp_form_pageswitch_42' => 'continue'], $manager->getDataOfAllSteps()->getAllSubmitted()); $this->assertSame(['label1' => 'foobar'], $manager->getDataOfAllSteps()->getAllLabels()); } public function testDataIsStoredProperlyAndDoesAdjustHookParametersOnLastStep(): void { $stepData = StepData::create(0); - $stepData = $stepData->withSubmitted(new ParameterBag(['submitted1' => 'foobar', 'mp_form_pageswitch' => 'continue'])); + $stepData = $stepData->withSubmitted(new ParameterBag(['submitted1' => 'foobar', 'mp_form_pageswitch_42' => 'continue'])); $stepData = $stepData->withLabels(new ParameterBag(['label1' => 'foobar'])); $stepData2 = StepData::create(1); - $stepData2 = $stepData2->withSubmitted(new ParameterBag(['submitted2' => 'foobar', 'mp_form_pageswitch' => 'continue'])); + $stepData2 = $stepData2->withSubmitted(new ParameterBag(['submitted2' => 'foobar', 'mp_form_pageswitch_42' => 'continue'])); $initialData = (new StepDataCollection())->set($stepData)->set($stepData2); $storage = $this->createStorage($initialData); @@ -80,7 +80,7 @@ public function testDataIsStoredProperlyAndDoesAdjustHookParametersOnLastStep(): $listener = new PrepareFomDataListener($factory, $this->createMock(RequestStack::class), $this->createMock(FileUploadNormalizer::class)); - $submitted = ['submitted3' => 'foobar', 'mp_form_pageswitch' => 'continue']; + $submitted = ['submitted3' => 'foobar', 'mp_form_pageswitch_42' => 'continue']; $labels = []; $fields = []; $files = [];