WordPress.org

Make WordPress Core

Opened 4 weeks ago

Closed 15 hours ago

#44396 closed defect (bug) (fixed)

Inconsistent use of blogname and sitename in Privacy emails

Reported by: desrosj Owned by: flixos90
Milestone: 4.9.8 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: fixed-major
Focuses: multisite Cc:

Description

In the core privacy email notifications, there is inconsistent use of get_option( 'blogname' ) and get_option( 'sitename' ).

In _wp_privacy_send_erasure_fulfillment_notification() and _wp_privacy_send_request_confirmation_notification(), the $email_data array has an index for sitename that is set to blog name. The subject is also set to blog name.

In wp_send_user_request(), the $email_data array has an index for site_name that is set to the site name if on multisite, and blog name for single sites. Further down in the function, the blog name is always used for the email subject.

The value used for these should be consistent across all privacy emails.

Attachments (5)

44396.diff (791 bytes) - added by subrataemfluence 4 weeks ago.
Proposed patch
44396-2.diff (815 bytes) - added by subrataemfluence 4 weeks ago.
Missed ENT_QUOTES. Fixed.
44396.2.diff (4.3 KB) - added by flixos90 3 weeks ago.
44396.3.diff (7.4 KB) - added by desrosj 3 weeks ago.
44396.4.diff (1.3 KB) - added by desrosj 11 days ago.

Download all attachments as: .zip

Change History (22)

@subrataemfluence
4 weeks ago

Proposed patch

#1 @subrataemfluence
4 weeks ago

  • Keywords has-patch added; needs-patch removed

@subrataemfluence
4 weeks ago

Missed ENT_QUOTES. Fixed.

@flixos90
3 weeks ago

#2 @flixos90
3 weeks ago

I agree we should be consistent, my suggestion would be to use single-site terminology throughout. Even before the privacy-related changes in 4.9.6, there were inconsistencies, for example in send_confirmation_on_profile_email() which used the network name in one place, but the site name for another place, both affecting the same email.

Since all of this so far does not really support a multisite-context yet, I think it makes sense to only use site scope at this point. Once we move forward with network-wide privacy control, we can enhance it as needed (for example there could be a parameter $network_wide for those emails, or similar).

Last but not least, this context issue does not only affect the site name, but also the URL.

In 44396.2.diff I use get_option( 'blogname' ) and home_url() throughout (instead of get_site_option( 'site_name' ) and network_home_url() for the network). I also changed a variable name from $blogname to $sitename since we shouldn't be using the "blog" terminology any more, and that variable is being publicly exposed via a filter and its docs.

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


3 weeks ago

#4 @desrosj
3 weeks ago

  • Milestone changed from Awaiting Review to 4.9.7

Related: #43822, #43738.

@desrosj
3 weeks ago

#5 @desrosj
3 weeks ago

44396.3.diff has some minor updates to ensure get_option( 'blogname' ) is only called once per function, and that it is always passed through wp_specialchars_decode().

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


2 weeks ago

#7 @flixos90
2 weeks ago

  • Owner set to flixos90
  • Status changed from new to reviewing

#8 @flixos90
2 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 43388:

Privacy: Use consistent values for the site name and URL used in notification emails.

The functions send_confirmation_on_profile_email(), _wp_privacy_send_request_confirmation_notification(), _wp_privacy_send_erasure_fulfillment_notification(), and wp_send_user_request() all include a title and URL indicating the current site. However, so far they have dealt with those values inconsistently, sometimes using the site values, other times using the network values if in a multisite. This changeset ensures that only the current site is taken into account in all cases and that special characters in the site name are consistently decoded.

Props subrataemfluence, desrosj.
Fixes #44396.

#9 @flixos90
2 weeks ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

This needs to be backported to 4.9.7.

#10 @pento
12 days ago

  • Keywords needs-patch added; has-patch fixed-major removed

This is causing Travis failures.

#11 @flixos90
12 days ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 43390:

Tests: Fix failing test after [43388].

Fixes #44396.

#12 @flixos90
12 days ago

  • Keywords fixed-major added; needs-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Both [43388] and [43390] need to be backported to 4.9.7.

#13 @ocean90
11 days ago

  • Milestone changed from 4.9.7 to 4.9.8

4.9.7 has been released, moving to next milestone.

#14 @desrosj
11 days ago

  • Keywords has-patch commit added

Noticed one more inconsistency in wp-admin/includes/file.php. The email linking to the user's personal data export file is sent in wp_privacy_send_personal_data_export_email(). 44396.4.diff has a fix for that.

@desrosj
11 days ago

#15 @flixos90
9 days ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 43435:

Privacy: Fix a further inconsistency of site name and URL usage in notification emails.

This is a follow-up to [43388].

Props desrosj.
Fixes #44396.

#16 @flixos90
9 days ago

  • Keywords has-patch commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

[43388], [43390], and [43435] all need to be backported to 4.9.8.

#17 @SergeyBiryukov
15 hours ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 43459:

Privacy: Use consistent values for the site name and URL used in notification emails.

The functions send_confirmation_on_profile_email(), _wp_privacy_send_request_confirmation_notification(), _wp_privacy_send_erasure_fulfillment_notification(), and wp_send_user_request() all include a title and URL indicating the current site. However, so far they have dealt with those values inconsistently, sometimes using the site values, other times using the network values if in a multisite. This changeset ensures that only the current site is taken into account in all cases and that special characters in the site name are consistently decoded.

Props subrataemfluence, desrosj.
Merges [43388], [43390], and [43435] to the 4.9 branch.
Fixes #44396.

Note: See TracTickets for help on using tickets.