Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Fixes errors that occur when the same form is used multiple times on the same page #247

Closed
wants to merge 1 commit into from

Conversation

maechler
Copy link
Contributor

There are currently some issues when using the same form multiple times on the same page (tested with TYPO3 v8.7.10 and powermail v5.5.0):

  • On server side validation errors, all the forms with the same uid get filled in and validated
  • When the form is successfully submitted, the success message always appears in the first form
  • The emails are always sent to the recipients of the first form on the page

This pull request fixes the issues the following way:

  • Adding an additional hidden field __ttcontentuid to the form containing the content element uid
  • Checking the POST parameter __ttcontentuid in FormController->forwardIfTtContentUidDoesNotMatch and forwarding to formAction if the uids do not match
  • Checking for the correct content uid in the PrefillViewHelpers and only prefill from GET / POST variables if the content uids do match
  • Inserting the Ajax response according to the attribute data-powermail-ttcontentuid instead of data-powermail-form

I am storing the data array of the current content object into GLOBALS['TSFE']->applicationData for two reasons (https://github.com/maechler/powermail/blob/develop/Classes/Controller/FormController.php#L478-L496):

  1. The data array gets cleared when errorAction is called after the validation failed. The errorAction redirects to the referring action formAction and thus the data array is missing when formAction wants to render the template. That means that the hidden field __ttcontentuid can not be filled in correctly when there are server side validation errors.
  2. We also need this information in the PrefillViewHelpers, like this it is easily accessible. Another way would be to pass the data down from Form.html to the field templates (e.g. Input.html), but that would have a bigger impact on existing code.

If you have any suggestions to improve the code I would be happy to update the pull request.

@einpraegsam
Copy link
Collaborator

This is really a large change in powermail. At the moment it's a missing feature to have the same form twice on the same page, even if it's possible to use multiple powermail plugins on the same page.
Because it's a big change, I'm not sure if I want to implement it with the next major update or not or only if I do some small refactorings.
For now: I will let this PR open. Probably some other admins can use and report feedback.

@maechler
Copy link
Contributor Author

I see that this is a big change. However I think that this pull request changes as little as possible to make it work. I would be very happy if this or a similar fix finds its way into the core.

@macjohnny
Copy link
Contributor

I think this change looks good and has been well-tested.

@macjohnny
Copy link
Contributor

any update on this?

@mhirdes
Copy link
Contributor

mhirdes commented Dec 23, 2019

I recognized the same problem on a customer page. Will this be fixed in the future?

@maechler
Copy link
Contributor Author

@mhirdes Applying the changes from my pull request should solve the issue, although I have not tested it with the most recent version of powermail. However it would still be nice if this fix or something similar would find its way into powermail.

@einpraegsam einpraegsam force-pushed the develop branch 4 times, most recently from a31ca4e to 43b5957 Compare July 8, 2022 10:49
@mschwemer mschwemer deleted the branch in2code-de:master December 28, 2023 10:19
@mschwemer mschwemer closed this Dec 28, 2023
@mschwemer mschwemer reopened this Dec 28, 2023
@mschwemer mschwemer changed the base branch from develop to master October 2, 2024 13:32
@mschwemer
Copy link
Collaborator

I know 6 1/2 years are a looong, loooong time from the start. 🙈 I am currently cleaning things up.

@mhirdes @maechler @macjohnny

Is this still an issue? If yes, would you mind to rebase the PR onto current master? If not, just close the PR or leave a short comment here.

TIA

@mschwemer
Copy link
Collaborator

Sorry for bugging ... there is #664 which is on an newer codebase.

Closing it

@mschwemer mschwemer closed this Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants