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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 30, 2024
+4 −2
Merged

Fix missing data in email submissions #55691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/block-library/src/form/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Copy link
Contributor

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?

Copy link
Member Author

@aristath aristath Dec 1, 2023 •
edited
Loading

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. 😃

Copy link
Contributor

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 :)

foreach ( $params as $key => $value ) {
if ( in_array( $key, $skip_fields, true ) ) {
continue;
Expand All @@ -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'] ),
Copy link
Contributor

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct

__( 'Form submission', 'gutenberg' ),
$content
);
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/form/view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

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?

Copy link
Member Author

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.

formData.formAction = form.action;

try {
const response = await fetch( wpBlockFormSettings.ajaxUrl, {
Expand Down
Loading