-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix missing data in email submissions #55691
Conversation
Size Change: +19 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love to know what was wrong before, in the code - I understand the email was not sent, why?. The code changes are not obvious to me :P)
email forms have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the explanation, left some comments around some code changes.
@@ -20,6 +20,8 @@ document.querySelectorAll( 'form.wp-block-form' ).forEach( function ( form ) { | |||
formData.formAction = form.action; | |||
formData._ajax_nonce = wpBlockFormSettings.nonce; | |||
formData.action = wpBlockFormSettings.action; | |||
formData._wp_http_referer = window.location.href; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overriding _wp_http_referer
like this makes me anxious. Why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data is not actually sent with the AJAX request... so we're just adding it here.
@@ -109,7 +109,7 @@ function block_core_form_send_email() { | |||
|
|||
// Send the email. | |||
$result = wp_mail( | |||
str_replace( 'mailto:', '', $params['wp-email-address'] ), | |||
str_replace( 'mailto:', '', $params['formAction'] ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like the main bug fixed here, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct
@@ -96,7 +96,7 @@ function block_core_form_send_email() { | |||
'<a href="' . esc_url( get_site_url( null, $params['_wp_http_referer'] ) ) . '">' . get_bloginfo( 'name' ) . '</a>' | |||
); | |||
|
|||
$skip_fields = array( 'formAction', '_ajax_nonce', 'action' ); | |||
$skip_fields = array( 'formAction', '_ajax_nonce', 'action', '_wp_http_referer' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe too late but it'd be great if we could instead of keeping alist of fields to skip, find a way to identify what to include - like if we had a prefix for all fields that should be emailed or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the args that need to be ignored are finite and known... That's why I thought it best to keep it clean and not add prefixes to the other ones. 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the args that need to be ignored are finite and known
Today! But one day some hidden thing _don't_email_me will show up and be emailed :)
c566fbe
to
5caf443
Compare
I am looking forward to getting this one merged. As I will then be able to test out the Forms block. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's get this in as is to enable this functionality now in the experiment.
5caf443
to
81784dd
Compare
Is there a fix for the Chrome warning: "This form is not secure. Auto-fill has been turned off" (Mixed content: ... |
What?
Fixes an issue with form submissions from the experimental form block, when using the email method.
Why?
Because emails were not being sent when submitting a form
How?
Adds missing data to the script, which then lets the form submission email to be properly sent.
Testing Instructions
type="submit"
for buttons #55690. If that PR doesn't get merged before this one, then you'll need to manually change the type of the button tosubmit
from your browser's tools, so the form can be properly submitted)Testing Instructions for Keyboard
Not applicable