Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#44192 closed defect (bug) (fixed)

Title of Privacy Policy Page not used on login page

Reported by: ov3rfly's profile Ov3rfly Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 4.9.8 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch has-unit-tests has-screenshots needs-testing
Focuses: Cc:

Description

  1. Have a page with some title like "Imprint and Privacy Policy"
  2. Set as "Privacy Policy Page"

Current Behaviour:

  1. Link on login page does not use the page title.

Expected Behaviour:

  1. Link on login page uses the page title.

Attachments (4)

44192.diff (1.6 KB) - added by desrosj 6 years ago.
Changes get_the_privacy_policy_link() link text to be the selected Privacy Policy page's post title. Includes unit test.
44192-login-page.PNG (18.3 KB) - added by ianbelanger 6 years ago.
Example of login page Privacy Policy text
44192.2.diff (2.5 KB) - added by desrosj 6 years ago.
44192.3.diff (3.0 KB) - added by birgire 6 years ago.

Download all attachments as: .zip

Change History (25)

#1 @knutsp
6 years ago

  • Keywords reporter-feedback added

Which login page? Do you mean wp-login.php or other?

#2 @Ov3rfly
6 years ago

  • Keywords reporter-feedback removed

The login and registration pages which are mentioned in the new Privacy Policy Page description.

#3 @knutsp
6 years ago

  • Keywords 2nd-opinion added

Confirmed on wp-login.php.

The anchor text is "Privacy Policy" or the localized version (translation) of this string, regardless the title of the page.

Should this be fixed, or a "wontfix"?

#4 @Ov3rfly
6 years ago

As already explained in the example "Imprint and Privacy Policy", the linked page might contain more than a privacy policy and pointing out the availibility of this content at the supplied link might be even legally necessary, e.g. in Germany.

#5 @pento
6 years ago

  • Keywords needs-patch added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to 4.9.7

I'm inclined to change the behaviour of get_the_privacy_policy_link() to use the post title, rather than a hardcoded string. As suggested, this allows easy customisation of the link text.

For localisation, the post is created with the post title translated to site's current language. For sites that support multiple languages, this should also fit into their content translation flow, as the post title will be localised along with the post content.

@desrosj
6 years ago

Changes get_the_privacy_policy_link() link text to be the selected Privacy Policy page's post title. Includes unit test.

#6 @desrosj
6 years ago

  • Keywords has-patch has-unit-tests added; needs-patch removed

#7 @desrosj
6 years ago

  • Keywords needs-testing added

#8 @ianbelanger
6 years ago

@desrosj Just tested your diff and all seems to be working fine for me. I changed my Privacy Policy page title and the hyperlinked text changed on the login page. I did not run the unit test though. Will do that later and report back. Screenshot coming.

@ianbelanger
6 years ago

Example of login page Privacy Policy text

#9 @ianbelanger
6 years ago

  • Keywords has-screenshots added; needs-testing removed

Unit tests passed, see results below:

..... 5 / 5 (100%)

You should really fix these slow tests (>150ms)...

  1. 465ms to run Tests_Link_GetThePrivacyPolicyLink:test_get_the_privacy_policy_link_should_return_valid_link_when_privacy_page_set

Time: 3.21 seconds, Memory: 24.00MB

OK (5 tests, 11 assertions)

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

#12 @desrosj
6 years ago

44192.2.diff adds a check for an empty page title and does not return a link if the privacy policy's page title is empty. This behavior may not be obvious for some users, but I am not sure we should assume "Privacy Policy" should be used in the absence of an actual page title.

#13 @ianbelanger
6 years ago

Just tested the latest patch and the Privacy Policy link is removed when
there is no title for the Privacy Policy page.

Nice work @desrosj

#14 @ocean90
6 years ago

  • Milestone changed from 4.9.7 to 4.9.8

4.9.7 has been released, moving to next milestone.

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


6 years ago

#16 @xkon
6 years ago

@desrosj everything looks good here :)

@birgire
6 years ago

#17 @birgire
6 years ago

44192.3.diff has just few suggestions, based on 44192.2.diff :

  • Escapes the privacy page title within the output of get_the_privacy_policy_link().
  • Adds the annotation @ticket 44192 for the new test method.
  • Adjusts to the new test method name and description.
  • As assertEmpty() results true for more than an empty string, e.g. null, false, I replaced it with assertSame( '', ... ), similar as in the existing test_get_the_privacy_policy_link_should_return_empty_string_when_privacy_page_not_set() method.
  • Combines assertions in test_get_the_privacy_policy_link_should_return_valid_link_when_privacy_page_set().

#18 @pbiron
6 years ago

  • Keywords needs-testing added

#19 @pbiron
6 years ago

Can this land in 4.9.8?

#20 @SergeyBiryukov
6 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 43506:

Privacy: Use the actual Privacy Policy page title in get_the_privacy_policy_link().

Props desrosj, birgire, ianbelanger, Ov3rfly.
Fixes #44192.

#21 @SergeyBiryukov
6 years ago

In 43507:

Privacy: Use the actual Privacy Policy page title in get_the_privacy_policy_link().

Props desrosj, birgire, ianbelanger, Ov3rfly.
Merges [43506] to the 4.9 branch.
Fixes #44192.

Note: See TracTickets for help on using tickets.