Make WordPress Core

Opened 23 months ago

Closed 17 months ago

Last modified 16 months ago

#56345 closed enhancement (fixed)

Add rel="privacy-policy' to get_the_privacy_policy_link function

Reported by: dshanske's profile dshanske Owned by: audrasjb's profile audrasjb
Milestone: 6.2 Priority: low
Severity: trivial Version: 4.9.6
Component: Privacy Keywords: good-first-bug has-patch has-privacy-review has-unit-tests has-screenshots commit needs-dev-note
Focuses: privacy Cc:

Description

This is noted in https://www.iana.org/go/rfc6903 . It would allow parsers to pull the link to the privacy policy.

Attachments (1)

Capture d’écran 2023-02-07 à 00.32.03.png (219.9 KB) - added by audrasjb 17 months ago.
Rel attribute also added to the nav menu item ✅

Download all attachments as: .zip

Change History (33)

#1 @davidbaumwald
23 months ago

  • Keywords good-first-bug added
  • Milestone changed from Awaiting Review to 6.1

#2 follow-up: @audrasjb
23 months ago

While I find it super useful, I'm not sure it's ready for 6.1, since it's only a RFC (Request for comments) for now, and yet not a part of the Link Types HTML specification or the WhatWG Link Types spec.

Maybe it would be better to wait a bit before implementing this in WordPress, because we the final recommandation can be slightly different than what's proposed in this RFC.

Version 0, edited 23 months ago by audrasjb (next)

#3 in reply to: ↑ 2 @dshanske
23 months ago

Replying to audrasjb:

While I find it super useful, I'm not sure it's ready for 6.1, since it's only a RFC (Request for comments) for now, and yet not a part of the Link Types HTML specification or the WhatWG Link Types spec.

Maybe it would be better to wait a bit before implementing this in WordPress, because the final recommandation can be slightly different than what's proposed in this RFC.

Unlikely as the proposal dates from 2013. I figured there was no other prior art, and I could see the utility of having privacy policy discovery.

This ticket was mentioned in PR #3135 on WordPress/wordpress-develop by bhavz-10.


22 months ago
#4

  • Keywords has-patch added

Add rel='privacy-policy' to get_the_privacy_policy_link function

Trac ticket: https://core.trac.wordpress.org/ticket/56345

#5 @bookwyrm
22 months ago

I've tested this patch and it works.

The only place in Core that the get_the_privacy_policy_link is used is on the /wp-login.php page and I see the attribute added there.

I did notice that when the Privacy Policy page is included in a Page List block, the rel="privacy-policy" attribute is not included in HTML output.

#6 @faisal03
21 months ago

I've also tested and it worked for me too. I've checked the /wp-login.php page and the rel="privacy-policy" was there.

Also checked what @bookwyrm mentioned above, the attribute does not show up when a Privacy Policy page is listed in the Page List block.

Do we need to show the attribute in that block? If yes, is it fine to load that attribute more than once on the same page?

#7 @JeffPaul
21 months ago

  • Keywords needs-privacy-review added

@audrasjb I get the point about waiting for this to get out of RFC but it seems that's been dragging on for quite some time. Perhaps if this did get into WP as part of a way to test out the viability of the RFC that would help get a large amount of support behind it such that it might more sooner become part of the spec?

#8 @peterwilsoncc
21 months ago

I dug in to this a little.

While looking in the GitHub issues for a standards spec, I saw a reference pointing to link types which uses privacy but doesn't document it in detail.

FWIW, the schema.org discussion is to use PrivacyPolicy so I suspect the inclusion of the word policy is more likey to get traction but that's pretty much a hunch.

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


21 months ago

#10 @paapst
21 months ago

  • Focuses privacy added
  • Keywords has-privacy-review added; needs-privacy-review removed

We discussed this in Core-Privacy. From a privacy point of view, we see no problems with getting this into WP as part of a way to test out the viability, as has been suggested by
@jeffpaul

#11 @audrasjb
21 months ago

My only concern would be to introduce a future backward compatibility issue if in the end, the specification doesn't validate the same rel attribute we use.

#12 @davidbaumwald
21 months ago

  • Milestone changed from 6.1 to 6.2
  • Type changed from defect (bug) to enhancement

Thinking more about the purpose of this change, it's clear that(at least as it's proposed currently) this is _additional_ functionality, not resolving a bug.

Given that, I'm updating the type to Enhancement and moving out of the 6.1 milestone now that it's entered the Beta phase.

I could see WP leading the way on uptake of this new functionality, so I'm interested in keeping the current momentum and moving to 6.2 rather than Future Release.

#13 @dshanske
21 months ago

That's my fault, I didn't notice I set it as defect. I agree it is enhancement. But, I doubt the proposal would move forward if it hasn't by now, so I agree we can trendset.

#14 @audrasjb
17 months ago

  • Keywords 2nd-opinion added
  • Owner set to audrasjb
  • Status changed from new to assigned

Hey there 👋

After speaking with some experts on this topic, I must confess that I have changed my mind on this. It would be very interesting if WordPress could took the lead on this topic to introduce this new rel attribute value.

In my opinion, privacy-policy is the most semantic wording, but privacy would work either way.

Anyway, first, the HTML document will still be W3C-compliant (the value of the rel attribute can be pretty much anything); and second, we can always change that value if the RFC comes out as than HTML specification, with a different implementation.

Last edited 17 months ago by audrasjb (previous) (diff)

#15 @audrasjb
17 months ago

Also, we may have to refine the patch to add it to the "classic" nav menu walker, and maybe open an issue on Gutenberg to make sure the attribute is added to the navigation block.

#16 @audrasjb
17 months ago

  • Keywords needs-unit-tests added

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


17 months ago

#18 @costdev
17 months ago

  • Keywords 2nd-opinion removed

This ticket was discussed during the recent bug scrub.

Both myself and @mukesh27 agree that privacy-policy is preferable. Removing 2nd-opinion.

@audrasjb After some testing, you're right - it looks like the "classic" Walker_Nav_Menu does need to be covered. I'll add a PR with this additional change shortly, and look into unit tests to cover both.

An issue still needs to be created on the Gutenberg repository to add this change to the Navigation Block.

Additional props to @mukesh27

This ticket was mentioned in PR #3950 on WordPress/wordpress-develop by @costdev.


17 months ago
#19

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

This covers get_the_privacy_policy_link() and Walker_Nav_Menu::start_el().
PHPUnit tests are included.

Trac ticket: https://core.trac.wordpress.org/ticket/56345

#20 @costdev
17 months ago

@audrasjb I've submitted PR 3950 to cover Walker_Nav_Menu::start_el() as well as add tests for this and for get_the_privacy_policy_link().

The PR is ready for review if you have time.

#21 @robinwpdeveloper
17 months ago

Test Report

This report validates that the indicated patch addresses the issue.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/3950

Environment

  • OS: macOS 11.2.3
  • Web Server: Nginx
  • PHP: 7.4.30
  • WordPress: 6.2-alpha-54642-src
  • Browser: Chrome 109.0.5414.119
  • Theme: Twenty Fifteen
  • Active Plugins:
    • No Plugins activated

Actual Results

  • ✅ Issue resolved with patch.

Additional Notes

  • Also run npm run test:php as per README.md and found similar results before and after applying patch.

Before: https://d.pr/i/30C9Z5
After: https://d.pr/i/0IvQcr

Supplemental Artifacts

Patch correctly add the attribute to login page.
Screenshot: https://d.pr/i/wKAPEj

@audrasjb
17 months ago

Rel attribute also added to the nav menu item ✅

#22 @audrasjb
17 months ago

  • Keywords has-screenshots added

#23 @costdev
17 months ago

  • Keywords commit added

Looks like PR 3950 is testing well. Adding for commit consideration.

#24 @kawserz
17 months ago

This report validates that the indicated patch addresses the issue.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/3950
Environment
OS: macOS 13.1
Web Server: Nginx
PHP: 8.0.27
WordPress: 6.2-alpha-54642-src
Browser: Chrome 109.0.5414.119
Theme: Twenty Twenty One
Active Plugins:
No Plugins activated
Actual Results

Issue resolved with patch.

Before: https://prnt.sc/vRqlwTmdgUla
After: https://prnt.sc/fk2z0kI5N5aB

#25 @audrasjb
17 months ago

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

In 55261:

Privacy: Add rel="privacy-policy" to the Privacy Policy link.

This changeset adds a rel="privacy-policy" attribute to user-facing links to the Privacy Policy of the website, when a privacy policy page is set and available. While this rel value is still a RFC of the Link Types HTML specification, this changeset helps to make Privacy Policy link more discoverable for user agents and HTML parsers.

Props dshanske, audrasjb, bhavz-10, bookwyrm, faisal03, JeffPaul, peterwilsoncc, paapst, davidbaumwald, costdev, robinwpdeveloper, kawserz.
Fixes #56345.

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


17 months ago

#28 @milana_cap
16 months ago

  • Keywords add-to-field-guide added

#29 follow-up: @audrasjb
16 months ago

  • Keywords needs-dev-note added

I'll write a small section about this change either in the Misc changes dev note or in a single one.

#30 in reply to: ↑ 29 @milana_cap
16 months ago

Replying to audrasjb:

I'll write a small section about this change either in the Misc changes dev note or in a single one.

Thank you

#31 @milana_cap
16 months ago

  • Keywords add-to-field-guide removed

#32 @audrasjb
16 months ago

In 55485:

Docs: Add missing 6.2.0 @since mention in get_the_privacy_policy_link().

Follow-up to #55261.

See #56792, #56345.

Note: See TracTickets for help on using tickets.