Make WordPress Core

Opened 6 years ago

Last modified 3 years ago

#44084 assigned defect (bug)

Privacy: The personal data export file should be generated in the user's locale

Reported by: coreymckrill's profile coreymckrill Owned by: garrett-eclipse's profile garrett-eclipse
Milestone: Future Release Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: needs-unit-tests needs-patch
Focuses: Cc:

Description

Similar to #43985, the HTML file containing the personal data export should be translated into the locale the user has selected for their profile, with a fallback to the site's locale.

Attachments (2)

file.diff (715 bytes) - added by vanyukov 6 years ago.
44084.diff (798 bytes) - added by desrosj 6 years ago.

Download all attachments as: .zip

Change History (37)

#1 @iandunn
6 years ago

  • Component changed from General to Administration
  • Milestone changed from Awaiting Review to 4.9.7
  • Status changed from new to assigned
  • Version set to trunk

#2 @iandunn
6 years ago

It'd be nice to get this in 4.9.6, but at this point it doesn't seem like there's enough time. cc @desrosj just in case you think it should be there.

#3 @desrosj
6 years ago

@iandunn I agree. Would be great, but I don't think there is enough time to test this properly. If a committer feels strongly about this for 4.9.6, they are more than welcome to see it through.

#4 @desrosj
6 years ago

  • Component changed from Administration to Privacy

Moving to the new Privacy component.

#5 @desrosj
6 years ago

  • Version changed from trunk to 4.9.6

Marking privacy bugs as introduced in 4.9.6.

#6 @desrosj
6 years ago

  • Milestone changed from 4.9.7 to 4.9.8

Moving all tickets in 4.9.7 to 4.9.8.

#7 @sebakurzyn
6 years ago

  • Resolution set to worksforme
  • Status changed from assigned to closed

I already tested this bug and everything works correctly. Tested on Duch translation, single site. Email and export data have the correct language.

#8 @pento
6 years ago

  • Milestone 4.9.8 deleted

#9 @SergeyBiryukov
6 years ago

  • Milestone set to 4.9.8
  • Resolution worksforme deleted
  • Status changed from closed to reopened

Tested with the following configuration:

  • Site's default locale: English (en_US)
  • Administrator's locale: Russian (ru_RU)
  • Editor's locale: Spanish (es_ES)

When exporting Editor's personal data, both the confirmation email and the export file use the administrator's locale (Russian in my case). Per comment:9:ticket:43985, they should use the requester's locale instead (Spanish in my case), with a fallback to the site's default locale for visitors who don't have an account.

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

@vanyukov
6 years ago

#10 @vanyukov
6 years ago

  • Keywords has-patch added; needs-patch removed

Added a patch. Will update the locale if the email for data export is from a valid user

#11 @desrosj
6 years ago

  • Keywords needs-testing added; gdpr removed
  • Milestone changed from 4.9.8 to 4.9.9

#12 @desrosj
6 years ago

  • Keywords needs-unit-tests added

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


6 years ago

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


6 years ago

@desrosj
6 years ago

#15 @desrosj
6 years ago

44084.diff is a different approach than file.diff.

It utilizes the user ID stored in the WP_User_Request object to prevent an unnecessary database query in get_user_by(). It also switches to the site's default locale if the current user is using a different locale, and switches back after the file is done generating.

There are a few strings not included in the actual file that are still not translated:

  • the stripped email, which has @ replaced with -at-.
  • the file basename, which is wp-personal-data-file-$stripped_email.

These two strings should probably be translated as well. But proper sanitization will need to be added for filenames with non-English characters.

Another situation to consider. An administrator can click a link to download the data export file in the admin. When that is clicked, a fresh file is generated. With 44084.diff, the file will always be in the user's selected locale. Should the file ever be generated in the administrator's locale instead?

Tests for this change should be added to the test class being worked on in #44233. I can add those once that is committed.

Last edited 6 years ago by desrosj (previous) (diff)

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


6 years ago

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


6 years ago

#18 @idea15
6 years ago

  • Owner set to javorszky
  • Status changed from reopened to assigned

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


6 years ago

This ticket was mentioned in Slack in #core by desrosj. View the logs.


6 years ago

#21 @desrosj
6 years ago

  • Keywords needs-refresh added
  • Owner changed from javorszky to desrosj

After discussion in yesterday's #core-privacy chat, it was determined that the data should be exported as follows:

  • If the user's locale matches the site's default locale, only one export file in that locale will be generated.
  • If the user's locale does not match the site's default locale, two files will be added to the ZIP file (one in each locale).
  • If the administrator's locale does not match either of these, no additional action will be performed.

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


6 years ago

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


5 years ago

#24 @pento
5 years ago

  • Milestone changed from 4.9.9 to Future Release

This ticket was mentioned in Slack in #core-privacy by webdevlaw. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

#28 @desrosj
4 years ago

  • Owner desrosj deleted

#29 @garrett-eclipse
4 years ago

  • Milestone changed from Future Release to 5.5
  • Owner set to garrett-eclipse

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


4 years ago

#31 @johnbillion
4 years ago

  • Keywords needs-patch added; has-patch needs-testing needs-refresh removed

Needs a new patch as the proposed approach to this task has changed.

#32 @apedog
4 years ago

This ticket might be related. A get_admin_locale() option that is independent of the Administrator's personal-preference user locale and independent of the front-facing site locale.

#49971

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


4 years ago

#34 @desrosj
4 years ago

  • Milestone changed from 5.5 to 5.6

With no activity in 5 weeks and beta 3 later today, I am going to punt this one. If someone can create a well rounded patch before RC1 (1 week from today), then it can be reconsidered.

#35 @garrett-eclipse
3 years ago

  • Milestone changed from 5.6 to Future Release
Note: See TracTickets for help on using tickets.