#56345 closed enhancement (fixed)
Add rel="privacy-policy' to get_the_privacy_policy_link function
Reported by: | dshanske | Owned by: | 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)
Change History (33)
#3
in reply to:
↑ 2
@
2 years 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.
2 years 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
@
2 years 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
@
2 years 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
@
2 years 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
@
2 years 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.
2 years ago
#10
@
2 years 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
@
2 years 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
@
2 years 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
@
2 years 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
@
21 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.
#15
@
21 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.
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
21 months ago
#18
@
21 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.
21 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
@
21 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
@
20 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
#23
@
20 months ago
- Keywords commit added
Looks like PR 3950 is testing well. Adding for commit
consideration.
#24
@
20 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
@audrasjb commented on PR #3950:
20 months ago
#26
Committed in https://core.trac.wordpress.org/changeset/55261
This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.
20 months ago
#29
follow-up:
↓ 30
@
20 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.
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.