Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#43620 closed enhancement (fixed)

Privacy Policy page design

Reported by: melchoyce's profile melchoyce Owned by: xkon's profile xkon
Milestone: 4.9.6 Priority: normal
Severity: normal Version:
Component: Privacy Keywords: gdpr has-patch needs-testing
Focuses: ui, administration Cc:

Description (last modified by ocean90)

Related to #43491 and #43473, this ticket addresses the design of the Privacy Policy page.

This design is a riff off of ticket:43435:edit-privacy-policy-page.png.

The draft has three main pieces:

  • Plugin Policies:
    • This is a collapsible box that lives above the Editor.
    • The header is styled like a warning notice, to attract attention to the text. The header contains an explanation of what the box contains and what you're supposed to do with it.
    • Each plugin providing a privacy policy will have a blurb and a "copy" button.
    • Question: Should the copy button copy text to the editor (I'd relabel it to "add to page page" or something), or just to your clipboard?
    • Question: Should there be some way to collapse or hide individual plugin blurbs?
  • Core Policies:
    • Core policies are automatically inserted into the page content.
    • Question: Do we see us making changes to this text between the first GDPR release, and when Gutenberg is released?
  • Confirmation:
    • This is inspired by the "Are you sure, [Author]?" confirmation the wordpress.org/news posts have.
    • The point of this extra step is to encourage people to review their privacy policy before publishing it.
    • When you check the box, the Publish button goes from disabled to enabled.
    • Question: Is this overkill?

Please note that none of the UI copy is final.

Attachments (32)

Privacy Policy: Classic Editor.png (470.5 KB) - added by melchoyce 6 years ago.
Privacy Policy: Classic Editor: Copy Updated 1.png (480.6 KB) - added by melchoyce 6 years ago.
Privacy Policy: Classic Editor: Copy Updated 2.png (486.9 KB) - added by melchoyce 6 years ago.
43620.diff (19.6 KB) - added by azaozz 6 years ago.
one.png (46.5 KB) - added by azaozz 6 years ago.
two.png (52.6 KB) - added by azaozz 6 years ago.
three.png (50.0 KB) - added by azaozz 6 years ago.
notice.png (8.0 KB) - added by azaozz 6 years ago.
43620.jq.showhide.gif (727.4 KB) - added by xkon 6 years ago.
43620.1.diff (3.5 KB) - added by xkon 6 years ago.
43620.1.gif (591.0 KB) - added by xkon 6 years ago.
43620.2.preview.gif (1.4 MB) - added by xkon 6 years ago.
43620.3.diff (10.1 KB) - added by xkon 6 years ago.
43620.4.diff (10.3 KB) - added by xkon 6 years ago.
collapsed.png (39.6 KB) - added by azaozz 6 years ago.
expanded-1.png (40.0 KB) - added by azaozz 6 years ago.
expanded-2.png (42.1 KB) - added by azaozz 6 years ago.
expanded-3.png (42.4 KB) - added by azaozz 6 years ago.
43620-url-filter.diff (989 bytes) - added by iandunn 6 years ago.
Add privacy_policy_url filter
43620.policy_url.policy_link.diff (1.6 KB) - added by xkon 6 years ago.
43620.policy_url.policy_link_2.diff (1.8 KB) - added by xkon 6 years ago.
43620_convert_to_metabox_for_classiand_gutenberg.gif (3.0 MB) - added by xkon 6 years ago.
43620_convert_to_metabox_for_classiand_gutenberg.diff (5.0 KB) - added by xkon 6 years ago.
43620_convert_to_metabox_for_classiand_gutenberg_2.diff (5.0 KB) - added by xkon 6 years ago.
43620.policy_url.policy_link_3.diff (2.6 KB) - added by iandunn 6 years ago.
URL string typecast, echo wrapper, minor cleanup
43620.policy_url.policy_link_4.diff (4.6 KB) - added by iandunn 6 years ago.
Only add before/after if link is truthy, add unit tests
43620.5.diff (8.5 KB) - added by azaozz 6 years ago.
screen.png (35.8 KB) - added by azaozz 6 years ago.
43620.6.diff (2.9 KB) - added by azaozz 6 years ago.
dismiss removed policy text.jpg (28.4 KB) - added by mnelson4 6 years ago.
It's nice knowing about plugins/themes that were removed and seeing their old privacy policy content for a while, but not forever. Eg, I activated then deactivated a plugin, but it's privacy policy content is stuck in there forever. It would be nice if a privacy policy for a removed plugin could be dismissed by pressing an X next to it, or something like this.
Screen Shot 2018-04-27 at 3.43.23 PM.png (172.8 KB) - added by melchoyce 6 years ago.
43620.fix-postbox-max-height.diff (310 bytes) - added by xkon 6 years ago.

Change History (111)

#1 follow-up: @allendav
6 years ago

Thank you @melchoyce ! At the top of the mockup you say “reviewing… making any necessary edits… and then copying and pasting” - where will the user edit? (The order suggests the suggested policies meta box allows for editing _before_ it is copied and pasted.)

#2 @allendav
6 years ago

Also, it might be possible for us to determine internally when a plugin has provided a change for their suggested policy. For the initial implementation, it might be helpful to display a "Updated on {DATE}" in the context of a plugin's row - what do you think @melchoyce ?

(Down the road we could get fancy and do diffs or something, but I think all we need to do for the initial implementation is note when the suggested policy for a plugin last changed.)

#3 in reply to: ↑ 1 @melchoyce
6 years ago

Replying to allendav:

At the top of the mockup you say “reviewing… making any necessary edits… and then copying and pasting” - where will the user edit? (The order suggests the suggested policies meta box allows for editing _before_ it is copied and pasted.)

I'd expect users to make edits after pasting into the editor, so we'll want to update the copy to make this more clear.

Also, it might be possible for us to determine internally when a plugin has provided a change for their suggested policy. For the initial implementation, it might be helpful to display a "Updated on {DATE}" in the context of a plugin's row - what do you think @melchoyce ?

Yeah, let me try mocking this up.

#4 follow-up: @melchoyce
6 years ago

Okay, two ideas:

I think the first version would be more informative, but if it's not feasible, the second solution would work okay.

#5 in reply to: ↑ 4 @dejliglama
6 years ago

Replying to melchoyce:

Okay, two ideas:

You are right, this would be the best approach since texts are copied and then (probably) changed to fit the language and style of the Website owner. Making it good to put emphasis on changes to plugin and WP core policy texts.

#6 @dejliglama
6 years ago

The height of each of the policy text bites from plugins could probably vary greatly in length.
I would probably be a good idea to have a "read more" or "see full text" button.

The copy btn should, of course, copy the full-length text.

#7 @ocean90
6 years ago

  • Description modified (diff)

#8 follow-up: @ocean90
6 years ago

With Gutenberg around the corner, is the current proposal actually the best approach here?

#9 @xkon
6 years ago

@ocean90 it's still unclear if GDPR related patches will be implemented pre or after Gutenberg so we figured we could tackle both just to be safe if possible.

We're checking everything out on the classic editor atm as we're all adding stuff in core with that being in there and when that's done we'll go on for a Gutenberg iteration if we actually make it first :D.

Unfortunately there are many things that change in #gdpr-compliance daily and this is one of them as well as we can't be sure when everything will be ready or which WordPress version we could bump into.

#10 in reply to: ↑ 8 @melchoyce
6 years ago

Replying to dejliglama:

The height of each of the policy text bites from plugins could probably vary greatly in length.
I would probably be a good idea to have a "read more" or "see full text" button.

The copy btn should, of course, copy the full-length text.

Good idea 👍

Replying to ocean90:

With Gutenberg around the corner, is the current proposal actually the best approach here?

As @xkon mentioned, there's a solid chance this will be released before Gutenberg. I did start exploring a Gutenberg version, though: https://marvelapp.com/3b5j6j4/screen/40182175

#11 @azaozz
6 years ago

The height of each of the policy text bites from plugins could probably vary greatly in length.
I would probably be a good idea to have a "read more" or "see full text" button.

Yeah, I've started working on this and expecting the text from each plugin to be somewhere between three to ten paragraphs. On average on most sites there will probably be five plugins that will output such text.

Nevertheless I don't think we should make the UI "the box of 1000 clicks" with accordions, sliders, tabs, or any other "show-me/hide-me" gizmos. A scrollbar so the user can scroll through all output would be enough imho. The simpler the UI -- the better :)

#12 @melchoyce
6 years ago

Yeah, the only potential problem with scrollbars is that's it's easy to get trapped in them or scroll by accident when you're trying to scroll the page itself.

@rianrietveld, if you have a moment to chime in from an accessibility perspective, which do you think will be better for folks using screen readers here — showing a bunch of copy in a list of scrollable containers, or having a "read more" link on each container?

#13 follow-up: @rianrietveld
6 years ago

Hey Mel,

which do you think will be better for folks using screen readers here — showing a bunch of copy in a list of scrollable containers, or having a "read more" link on each container?

Whichever you choose: the text of the copy button or read more link should be unique (you can add the name of the plugin with screen-reader-text). Otherwise it's just a bunch of meaningless "copy" and "read more" buttons/links in the lists. And the message must be placed somewhere a screen reader noticed it before entering the content of the page, because this is unexpected behavior from the normal procedure.

From a usability/maintainable point of view, here is my proposal:
Above the title there is an explanation about this page and how it is generated.

In the content area a good default text is added on creation of the page (maybe reviewed per country?). Site owners can edit this to their liking.

At the bottom of the content there will an auto generated list of links, to all the Privacy Policy pages of the plugins on their sites. That way the page doesn't have to be updated when the plugins update their statement. Also: in the construction above content managers can edit the policies of the plugins, which may cause problems for the plugins if they are altered wrongly. By referring to a page of the plugin, the responsibility and text stays with the plugin owner.

Bonus: it's super accessible and doesn't involve a scrollbar or tons of reading from the site owner.

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

#14 in reply to: ↑ 13 @azaozz
6 years ago

Replying to rianrietveld:

Thanks for the review and the suggestions!

Above the title there is an explanation about this page and how it is generated.

Good idea imho.

In the content area a good default text is added on creation of the page (maybe reviewed per country?). Site owners can edit this to their liking.

Yes, the plan is to include the "default" text from WordPress when a new page is created. The site owners will also have the option to select an existing privacy policy page when they already have one.

At the bottom of the content there will an auto generated list of links, to all the Privacy Policy pages of the plugins on their sites. That way the page doesn't have to be updated when the plugins update their statement.

Thought of doing this but the problem will be that most privacy policies on plugins websites will not be translated. I'm not sure if that's acceptable. If it is, that'd be the easiest solution. Technically we can even add a "dynamic" list of links when outputting the privacy policy page on the front-end. Then the policy won't need to be edited at all when plugins are added, deleted or updated.

Also: in the construction above content managers can edit the policies of the plugins, which may cause problems for the plugins if they are altered wrongly. By referring to a page of the plugin, the responsibility and text stays with the plugin owner.

The site owners should always be responsible for their sites policies. We'll be getting into very murky waters if plugins "inherit" any legal responsibilities from the sites they are used on.

The idea for the above postbox is that plugins should add there "information that may need to be added to the site's policy", not completed individual policies for each plugin. Then the site owner will be able to use that information in creating their own privacy policy, and hopefully seek some legal advice about the whole policy content.

I'm also thinking if we should move the postbox under the editor and have a link that will scroll to it (and focus it) in the explanation above the title. It seems it is going to be under the editor in Gutenberg too.

This ticket was mentioned in Slack in #gdpr-compliance by xkon. View the logs.


6 years ago

@azaozz
6 years ago

#16 @azaozz
6 years ago

In 43620.diff - first run of the Privacy Policy postbox:

  • Caches the current suggested text in post_meta. Then compares it after every plugin activation or deactivation. If there are changes, outputs a warning (for admins only) with a link to edit the privacy policy page.
  • Introduces wp_add_privacy_policy_content( $plugin_name, $policy_text ) for plugins to add suggested text for inclusion in the site's privacy policy.
  • Introduces get_privacy_policy_url() to retrieve the URL to the privacy policy.

To highlight any changes the policy texts from plugins have three statuses:

  • Added (date).
  • Updated (date) green background.
  • Removed (date) red background.

These are added after a plugin that adds policy text is activated or deactivated and the text is compared with the cache. The statuses are removed after the policy page is saved/updated.

TODO:

  • Folding (somehow) of each policy text? With many plugins the text can get pretty long, and the scrolling may become annoying.
  • UI/behaviors review.

@azaozz
6 years ago

@azaozz
6 years ago

@azaozz
6 years ago

@azaozz
6 years ago

#17 @azaozz
6 years ago

In 42980:

Privacy: add a postbox that is shown when editing the privacy policy page, and where plugins and core will output suggested content and additional privacy info. First run.

Props melchoyce, azaozz.
See #43620.

#18 @xkon
6 years ago

@azaozz about the todo: folding that you have above I've been playing with some stuff but ended up on a super basic jQ iteration just for the sake of prevewing things. ( this is not for a patch ofc )

I think that one good spot would probably be a button though somewhere with the Copy one to Hide/Show more.

By adding a button we could:
1] trim the incoming text with php and just add an extra 'hidden' placeholder for the full text, to avoid UI flickering :)
2] avoid copying the 'Read More / Read Less' text when pressing Copy button

var desc_policy_text = $('.policy-text').find('p:first');

desc_policy_text.each(function(){
	$(this).append('...<a href="#" class="more">Read More</a><span><a href="#" class="less">Read Less</a><span>');
	$(this).siblings().toggle();
	$(this).find('a.less').toggle();
})

$('a.more').click(function(e){
	e.preventDefault();
	$(this).parent().siblings().toggle();
	$(this).toggle();
	$(this).siblings().find('a.less').toggle();
});

$('a.less').click(function(e){
	e.preventDefault();
	$(this).parent().parent().siblings().toggle();
	$(this).toggle();
	$(this).parent().parent().find('a.more').toggle();
});

Gif of this small jQ snippet following

Also @melchoyce do you think that we could go for an alternating background between each plugins privacy policy text or create a better division somehow between each text? Since everything is in a pretty in a scroll window the only things that differenatiate the text between policies are the border and the bold title. Sometimes it's a bit hard to see where one ends and the next one starts if the text is quite long.

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

#19 @melchoyce
6 years ago

@azaozz Can we remove the "Copy" button for plugins that remove their privacy policies?

Also @melchoyce do you think that we could go for an alternating background between each plugins privacy policy text or create a better division somehow between each text? Since everything is in a pretty in a scroll window the only things that differenatiate the text between policies are the border and the bold title. Sometimes it's a bit hard to see where one ends and the next one starts if the text is quite long.

Yeah — let's go with #f9f9f9 alternating with white to match the striping on list tables.

One small note about the above:

  • We should use &hellip; for the ellipsis
  • There should be a space between the ellipsis and "Read More"

Circling back to your feedback, @rianrietveld, I agree with you that we should add an explanation to the page. Thanks for chiming in — @azaozz @xkon can we make sure we implement @rianrietveld's accessibility points if we haven't already?

I also like this suggestion from @azaozz:

I'm also thinking if we should move the postbox under the editor and have a link that will scroll to it (and focus it) in the explanation above the title. It seems it is going to be under the editor in Gutenberg too.

#20 @azaozz
6 years ago

In 42985:

Fix typo in 'wp_get_default_privacy_policy_content' filter.

Props claudiu.
See #43620.

@xkon
6 years ago

@xkon
6 years ago

#21 @xkon
6 years ago

In 43620.1.diff

  • Moves the Policy text js into their own function($) in post.js .
  • Removes the Copy button for the text-removed divs.
  • Adds Read More / Read Less links.
  • Adds an extra policy-text-excerpt div that trims the policy text down to 400 chars.
  • Disables the Copy button by default and enables it when you click on 'Read More' so you can copy the full text.

On disabling the buttons by default:
The 'copy' works by selecting the div, since it's hidden if you have not pressed Read More you can't actually copy the contents so it results in an error or copying out whatever else :D. I'm not sure how to 'fix' this if there's even a fix for it so I just though of disabling the button thus 'forcing' people to review the full text as well before blindly copy pasting everything.

This ticket was mentioned in Slack in #themereview by xkon. View the logs.


6 years ago

#23 @desrosj
6 years ago

  • Milestone changed from Awaiting Review to 4.9.6
  • Owner set to xkon
  • Status changed from new to assigned

Moving to the 4.9.6 milestone after consensus was reached in the most recent GDPR chat (https://wordpress.slack.com/archives/C9695RJBW/p1524063200000304).

#24 @melchoyce
6 years ago

Two notes based on the above screenshot:

  1. The "Policy text added/removed" line should be 11px, #555D66, and font-weight: 600;.
  2. Seeing the "Policy removed" containers without the copy button now makes me think if the policy was removed, we should hide the entire content behind a read more link, with a label like "Read removed text."

@xkon
6 years ago

@xkon
6 years ago

#25 follow-up: @xkon
6 years ago

43620.3.diff continues on the last patch and moves the postbox underneath the Editor, moves the Copy button within the expanded policy text adds some minor UI fixes and Coding standards.

Now I'll say this just for a heads up. The Copy button that uses selectRange() seems seriously messed up to me and I can't find a way to make it copy only the 'content'. If you paste in the Text Editor you're fine, but if you paste on the Visual Editor you're getting the <div> holders as well and that might/will probably break up front-ends.

If anyone can help on that matter it'd be great, if not I'm still in favor of removing it completely instead of giving random headaches :) .

#26 in reply to: ↑ 25 @azaozz
6 years ago

Replying to xkon:

Now I'll say this just for a heads up. The Copy button that uses selectRange() seems seriously messed up to me and I can't find a way to make it copy only the 'content'. If you paste in the Text Editor you're fine, but if you paste on the Visual Editor you're getting the <div> holders as well and that might/will probably break up front-ends.

If anyone can help on that matter it'd be great, if not I'm still in favor of removing it completely instead of giving random headaches :)

Yeah, think we can "fold" the individual text sections by setting height: 100px; or similar on the wrapper div. That way we won't have to insert "foreign" elements in the middle of the text added by plugins. The part with the folding link can be absolute positioned to the bottom.

Also thinking the folding can work by adding and removing a class on the wrapper, no need of all the extra markup. I'll try that tomorrow :)

@xkon
6 years ago

#27 @xkon
6 years ago

@azaozz the fixed height might have caused issues on responsive things since the text would remain fluid.

Either way your 'position: absolute' triggered a nice thought and I figured out what the issue was. If we had extra html below the policy-text it was copying the tags also. If that <div> is last on the list the copy worked fine.

In 43620.4.diff I kept everything as on 43620.3 but re-arranged the html and added the absolute positioning to make the Copy working properly as well.

Everything seems ok here now and works in responsive as well without issues

#28 @azaozz
6 years ago

the fixed height might have caused issues on responsive things since the text would remain fluid.

Actually, exactly the opposite. If you hard-code the text "cut" position, it will make the containers taller on smaller screens. Also, why would we hard-code something in PHP when it can be done with CSS better? :) I'll try that and compare the results.

@azaozz
6 years ago

@azaozz
6 years ago

@azaozz
6 years ago

@azaozz
6 years ago

#29 @azaozz
6 years ago

In 42992:

Privacy: make the sections in the suggested privacy policy text postbox foldable. Add Read More/Read Less buttons. Fix copying of the suggested text by pressing the button.

Props melchoyce, xkon, azaozz.
See #43620.

@iandunn
6 years ago

Add privacy_policy_url filter

#30 follow-up: @iandunn
6 years ago

I think it'd be helpful to have a filter in get_privacy_policy_url(). My use case is that some Multisite networks would like to set a single policy for all sites, rather than having to manually create them per site or allowing individual site admins to modify it.

On a Multisite network like WordCamp.org, that's much more practical than manually creating hundreds of pages, and it allows us to ensure that all sites in the network have a consistent and GDPR-compliant policy.

43620-url-filter.diff is a first pass at that. Does anyone have any feedback on it?

#31 in reply to: ↑ 30 @azaozz
6 years ago

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

Replying to iandunn:

I think it'd be helpful to have a filter in get_privacy_policy_url().

Yep, the patch looks good :)

Was also thinking we may want to continue the trend of making two related template functions: one that returns the URL and another that echoes/outputs the link tag. That way we will be able to standardize the HTML, translate the link text in core, etc.

Something similar to get_edit_comment_link() and edit_comment_link(), get_previous_posts_link() and previous_posts_link(), etc.

#32 @birgire
6 years ago

The get_privacy_policy_url() uses get_permalink() that returns a string or false on failure:

* @return string|false The permalink URL or false if post does not exist.

so that means get_privacy_policy_url() can return both empty string or false on failure. But that's currently not reflected in the doc block of get_privacy_policy_url():

 * @return string The URL to the privacy policy page. Empty string if it doesn't exist.

Examples:

delete_option( 'wp_page_for_privacy_policy' );
var_dump( get_privacy_policy_url() );
string(0) ""

update_option( 'wp_page_for_privacy_policy', 0 );
var_dump( get_privacy_policy_url() );
string(0) ""

update_option( 'wp_page_for_privacy_policy', PHP_INT_MAX );
var_dump( get_privacy_policy_url() );
bool(false)

In general there's inconsistency between core get_*_url() functions, that they return either empty string or false on failure.

I think we should choose either false or an empty string here.

There exists a function since 4.7, with a familiar structure, that we could look into:

/**
 * Retrieve header video URL for custom header.
 *
 * Uses a local video if present, or falls back to an external video.
 *
 * @since 4.7.0
 *
 * @return string|false Header video URL or false if there is no video.
 */
function get_header_video_url() {
	$id  = absint( get_theme_mod( 'header_video' ) );
	$url = esc_url( get_theme_mod( 'external_header_video' ) );

	if ( $id ) {
		// Get the file URL from the attachment ID.
		$url = wp_get_attachment_url( $id );
	}

	/**
	 * Filters the header video URL.
	 *
	 * @since 4.7.3
	 *
	 * @param string $url Header video URL, if available.
	 */
	$url = apply_filters( 'get_header_video_url', $url );

	if ( ! $id && ! $url ) {
		return false;
	}

	return esc_url_raw( set_url_scheme( $url ) );
}

and template tags to use in themes:

/**
 * Display header video URL.
 *
 * @since 4.7.0
 */
function the_header_video_url() {
	$video = get_header_video_url();
	if ( $video ) {
		echo esc_url( $video );
	}
}

/**
 * Check whether a header video is set or not.
 *
 * @since 4.7.0
 *
 * @see get_header_video_url()
 *
 * @return bool Whether a header video is set or not.
 */
function has_header_video() {
	return (bool) get_header_video_url();
}

We could also consider:

function the_privacy_policy_url() {
	$privacty_policy_url = get_privacy_policy_url();
	if ( $privacty_policy_url ) {
		echo esc_url( $privacty_policy_url );
	}
}

function has_privacy_policy() {
	return (bool) get_privacy_policy_url();
}

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

#33 follow-up: @xkon
6 years ago

After going through all the themes in #43715 and re-checking the comments here ( was kinda late! ) I thought of taking a shot at what @azaozz proposed so, made 43620.policy_url.policy_link.diff that extends on 43620-url-filter.diff just in case I have it right.

This way I could go back into the themes and change everything basically to get_privacy_policy_link( '<span class="privacy-link-wrapper">', '</span>' ); if I'm correct :D

#34 in reply to: ↑ 33 @birgire
6 years ago

Replying to xkon:

After going through all the themes in #43715 and re-checking the comments here ( was kinda late! ) I thought of taking a shot at what @azaozz proposed so, made 43620.policy_url.policy_link.diff that extends on 43620-url-filter.diff just in case I have it right.

That looks good, I would though not name the function with get_*() if it will echo the output and not return it.

Sometimes there's $echo = true input argument, like we see in edit_term_link().

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

#35 follow-up: @xkon
6 years ago

In 43620.policy_url.policy_link_2.diff renamed get_privacy_policy_link() to the_privacy_policy_link() as it makes more sense I suppose and added the $echo = true for echo or return as @birgire mentioned in the previous comments.

#36 @xkon
6 years ago

43620_convert_to_metabox_for_classiand_gutenberg_2.diff makes some changes to convert the current postbox into a meta box so it can be used with Gutenberg. Not sure if it's too early for it and I don't know if it's a full-proof way of doing it so 2nd opinions are always welcome but it works pretty well as far as I've tested.

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

@iandunn
6 years ago

URL string typecast, echo wrapper, minor cleanup

#37 in reply to: ↑ 35 ; follow-up: @iandunn
6 years ago

43620.policy_url.policy_link_3.diff has a few iterations on the URL/link functions:

  • Typecast get_permalink() result to a string, to ensure a consistent return type.
  • Adds a separate echo function, instead of an $echo = true param. This is kind of subjective; both patterns exist in Core. To me, the former feels slightly more convenient for devs, and a bit more straightforward for both humans and linting tools. The function and filter names were updated as a result.
  • Minor alignment/standards/line wrapping/etc tweaks.

Does anyone have any feedback on that? If not, I think it's ready for commit.

#38 in reply to: ↑ 37 ; follow-up: @azaozz
6 years ago

Replying to iandunn:

Does anyone have any feedback on that? If not, I think it's ready for commit.

Looks good except perhaps return an empty string without the $before and $after when no link. Perhaps:

$link = apply_filters( 'the_privacy_policy_link', $link, $privacy_policy_url );

if ( $link ) {
  return $before . $link . $after;
}

return '';

#39 in reply to: ↑ 38 ; follow-up: @iandunn
6 years ago

Replying to azaozz:

Looks good except perhaps return an empty string without the $before and $after when no link.

Ah, good catch :)

I'll go ahead and commit that in order to make collaboration easier and unblock #43715, but we can continue discussing/iterating if anyone else has any feedback.

This ticket was mentioned in Slack in #gdpr-compliance by azaozz. View the logs.


6 years ago

#41 @mnelson4
6 years ago

WP_Privacy_Policy_Content::add_suggested_content() doesn't translate the string "WordPress". It seems it's usually translated in WP core when it's part of a phrase, but https://github.com/WordPress/wordpress-develop/blob/f12dec95fed27ad17c20ac0a92316534c40d8c54/src/wp-admin/includes/class-language-pack-upgrader.php#L364 specifically does NOT translate it.
IMO it should be translated (Marius Jensen pointed out some languages change it for pronunciation, and I'm not sure what non-romantic languages do with that string of romantic characters), but I think someone from the i18n team should probably confirm what's best.

This ticket was mentioned in Slack in #core-i18n by mnelson4. View the logs.


6 years ago

#43 in reply to: ↑ 39 ; follow-up: @birgire
6 years ago

Replying to iandunn:

I'll go ahead and commit that in order to make collaboration easier and unblock #43715, but we can continue discussing/iterating if anyone else has any feedback.

I would suggest adding numbered placeholders.

I've finished the unit-tests for both get_privacy_policy_url() and get_the_privacy_policy_link(). I've created the files:

tests/phpunit/tests/link/get-the-privacy-policy-link.php
tests/phpunit/tests/link/get-privacy-policy-url.php

but I'm scratching my head to what ticket I should add them.

This current ticket looks somewhat unrelated ;-)

What about creating a new one for these new functions as suggested by @xkon? That way it wouldn't be buried in an unrelated ticket ;-)

#44 follow-ups: @grapplerulrich
6 years ago

I was looking at the code of wp_add_privacy_policy_content( 'WordPress', $content ).

In class WP_Privacy_Policy_Content I found the variable $plugin_name and the documentation geared towards Plugins only. I find that mentioning only plugins in the documentations and the variables confusing that the function can only be used in plugins. This is not the case as wp_add_privacy_policy_content() is being used for WordPress itself.

I could imagine a scenario where a plugin may have multiple privacy policies depending on the features enabled. I feel that the adding privacy policy content should not be coupled so strongly just to plugins.

Instead of plugins terms like application or extensions could be used.

@iandunn
6 years ago

Only add before/after if link is truthy, add unit tests

#45 @tobifjellner
6 years ago

Yes, please make WordPress translateable. One example of a domain that DOES translate, or rather transcribe this name is https://hi.wordpress.org/

#46 in reply to: ↑ 43 @iandunn
6 years ago

Replying to birgire:

What about creating a new one for these new functions

Yeah, you're right. I picked this one since it's where get_privacy_policy_url() was introduced, and I initially thought this would just be adding a simple filter.

In hindsight, I should have created a new ticket. I've done that now, though: #43850

I'll reply to your other feedback there.

@azaozz
6 years ago

@azaozz
6 years ago

#47 @azaozz
6 years ago

43620.5.diff is based on 43620_convert_to_metabox_for_classiand_gutenberg_2.diff:

  • Moves the metabox under the editor (see screenshot).
  • Defines the new privacypolicy metabox in edit-form-advanced.php, together with the rest of the metaboxes.
  • Fixes the styling.

Still TODO/TBD: as the suggested text is under the editor, the privacy policy page looks exactly the same as any other page. May be somewhat confusing? Shall we add some sort of a message above the title (where all messages go) and have a "scroll to the metabox" link in it? Can also be most of the "help text" from the metabox:

The suggested content for your privacy policy is under the editor. It comes from plugins and themes you have installed. We suggest reviewing it then copying and pasting it into your privacy policy page.

where suggested content can be a link that will scroll the page to the metabox.

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

#48 @azaozz
6 years ago

In 42999:

Make the string WordPress translatable.

Props mnelson4.
See #43620.

#49 in reply to: ↑ 44 @postphotos
6 years ago

Replying to grapplerulrich:

In class WP_Privacy_Policy_Content I found the variable $plugin_name and the documentation geared towards Plugins only. I find that mentioning only plugins in the documentations and the variables confusing that the function can only be used in plugins. This is not the case as wp_add_privacy_policy_content() is being used for WordPress itself.

I could imagine a scenario where a plugin may have multiple privacy policies depending on the features enabled. I feel that the adding privacy policy content should not be coupled so strongly just to plugins.

Instead of plugins terms like application or extensions could be used.

Hi @grapplerulrich - @xkon pinged me on this and I want to add a few thoughts.

I agree with you both - $plugin_name is a little vague if a theme or core developer wanted to hook into this, and it's still not really addressing the extendability question: A forms plugin might have a myriad of innocuous questions that collection of could be pretty safe, but dealing with personal (email, phone, etc.) or financial data (credit cards as processed through a third-party, thinking about Gravity Forms for example) might have multiple versions of a policy based on what a user has already completed. While we don't have to build this complex tool, we should support this kind of flow by default.

Outside of the scope of this conversation - we should encourage creative usage of this for this kind of scenario: A user may not realize how sensitive the data they collect or process is.

I propose we rename $plugin_name to $policy_name (also matching the pattern of scope varible $policy_text): A slight difference, but makes this feel more holistic by terminology. (I feel the terms "extension" and "application" is too vague here, so call it what it is.)

I also suggest we register a $policy_kind to specify whether it's a theme, core or plugin making this request for future organization in UI if we ever choose to do so and is just a bit cleaner architecturally. :)

#50 @xkon
6 years ago

@postphotos Thanks for the input!

I'll add my thoughts as well after having that postbox in front of me for the last days while working on other things also:

What I think could work a bit better would be something like along what @postphotos said with the difference of keeping everything on it's own so something like:

Name (whoever adds the policy) - Policy Name (optional if it's for something specific) - Policy Text

Although I would prefer for everyone to use wp_add_privacy_policy_content() 1 time only and add as much text as they please in 1 hook there's no limit to how many times they want to call it at the moment so a plugin / theme might eventually send over more than 1 calls. For a thorough example let's take Tagregator (yay!) for instance :D

As a plugin it might send:

only 1 call to wp_add_privacy_policy_content
Tagregator - Connecting to this that this there, api blah blah

-OR-

wp_add_privacy_policy_content if instagram enabled
Tagregator - Instagram - Text

wp_add_privacy_policy_content if twitter enabled
Tagregator - Twitter - Text

So we might end up with multiple 'entries' from plugins etc.

I'd personally alter the call to accept: $name, $policy_name, $policy_text, the kind I don't know if we care much but if we do then that's an 'extra' var for it just in case we wan't to use it somehow.

Does that make sense? ( kinda early here :D )

--

cc @grapplerulrich / @azaozz

#51 @grapplerulrich
6 years ago

I like the change from $plugin_name to $policy_name. I am on the fence if an additional argument is needed for separate sections.

I find having both $name and $policy_name confusing. $type or $section maybe better.

As we were talking about organisational structure. How are the privacy policies ordered? Are they ordered in the order that they are registered? I think the WordPress privacy policy should always come to the top and the maybe the others should be order alphabetically making it easier to search through.

#52 @xkon
6 years ago

I'm just adding the extra argument as a thought so all Authors can understand what they have to output easily so "name - title(optional) - text" is required.

Giving you only a field 'policy_name' doesn't necessary mean that you have to tell me your plugin's name so I can understand where that policy is coming from as an admin that I might decide to even uninstall it after reading the policy. It might be 1 extra var but it will save a lot of trouble in the future imho. It's even easier than us grabbing the name automatically and end up having double text even. If for example $extension - $policy_name - $policy_text fits the description better then sure let's go for that, but this information should be there in the postbox one way or another imho.

#53 in reply to: ↑ 44 @azaozz
6 years ago

Replying to grapplerulrich:

In class WP_Privacy_Policy_Content I found the variable $plugin_name and the documentation geared towards Plugins only.

Ah, you mean theme authors will be jealous? :)

Joking aside, I agree this needs better docs.

I could imagine a scenario where a plugin may have multiple privacy policies depending on the features enabled.

Right, and this is specifically supported. A plugin or theme can add as many sections of suggested text as needed.

I propose we rename $plugin_name to $policy_name...

I'd personally alter the call to accept: $name, $policy_name, $policy_text...

Not sure this is a good idea. First, $policy_name is incorrect. The themes and plugins are not adding whole, complete privacy policies (think of the legal implications!), they are suggesting text that site admins may want to use in their sites' privacy policies :)

Then $plugin_name makes it very clear what is expected. It's not a "Privacy Policy Name" (do privacy policies have names?), it is the name of the plugin or theme that is adding the suggested content.

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

@azaozz
6 years ago

#54 @azaozz
6 years ago

In 43620.6.diff: better docs for wp_add_privacy_policy_content() and WP_Privacy_Policy_Content::add().

#55 @xkon
6 years ago

Regarging 43620.5.diff yes I think it'll be good to have a message above the editor since everything is moved under with a scroll-to link as well.

I feel that a smaller text more of like a non-dismisable notice ( if possible ) would be better as it would be first in screen as well not just above the editor. We already 'tell' them to review and make their own edits etc inside the postbox, I don't think there's a need of duplicating that imho.

As for the text maybe something like would be enough (?) (note I'm also adding the word activated since you might have a plugin installed but you won't see the policy until it's active as well) :

The <link>suggested content</link> from the plugins and themes you have installed and activated is under the editor.

#56 @azaozz
6 years ago

In 43003:

Privacy: add better docs for wp_add_privacy_policy_content() and WP_Privacy_Policy_Content::add().

See #43620.

@mnelson4
6 years ago

It's nice knowing about plugins/themes that were removed and seeing their old privacy policy content for a while, but not forever. Eg, I activated then deactivated a plugin, but it's privacy policy content is stuck in there forever. It would be nice if a privacy policy for a removed plugin could be dismissed by pressing an X next to it, or something like this.

#57 @xkon
6 years ago

@mnelson4 the policy texts are removed from the list when you Update the page. Imho it should stay that way as some users might hit Dismiss and never actually update their policy and forget about the changes.

In any case it's again 1 click away to remove all of them in 1 action ( maybe we should explain that ? )

Or at least I would suggest to leave it as is for now and maybe go for a change change in the future so we can see how this whole flow feels in live action and have some feedback also for it.

#58 @mnelson4
6 years ago

the policy texts are removed from the list when you Update the page.

oups, didn't notice that! Yeah that's fine then.

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


6 years ago

This ticket was mentioned in Slack in #gdpr-compliance by mnelson4. View the logs.


6 years ago

#61 follow-up: @melchoyce
6 years ago

⬆ Seeing a bunch of empty space in 43620.6.diff.

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

#62 in reply to: ↑ 61 @azaozz
6 years ago

Replying to melchoyce:

Uh, that's a bug... Will look tomorrow night or on Sunday.

This ticket was mentioned in Slack in #gdpr-compliance by desrosj. View the logs.


6 years ago

#64 follow-up: @xkon
6 years ago

43620.fix-postbox-max-height.diff changes the postbox height: 320 to max-height: 320 to address the white space issue if there aren't many incoming texts as seen above comment:62

@desrosj since we're keeping the postbox above the editor for now I think we're good with this as well.

When we move it 'underneath' the editor or go Gutenberg etc we'll probably open up new tickets for various discussions in here ( see https://core.trac.wordpress.org/ticket/43620#comment:47 for example ) - we will reference this ticket from the new when needed.

This ticket was mentioned in Slack in #gdpr-compliance by xkon. View the logs.


6 years ago

This ticket was mentioned in Slack in #gdpr-compliance by xkon. View the logs.


6 years ago

#67 in reply to: ↑ 64 @azaozz
6 years ago

Replying to xkon:

Thanks for the patch. Went with a bit different approach in r43052. The idea was that if there is only one section of text in the postbox, we shouldn't fold it.

#68 @SergeyBiryukov
6 years ago

In 43101:

Privacy: add a postbox that is shown when editing the privacy policy page, and where plugins and core will output suggested content and additional privacy info. First run.

Props melchoyce, azaozz.
Merges [42980] to the 4.9 branch.
See #43620.

#69 @SergeyBiryukov
6 years ago

In 43112:

Fix typo in 'wp_get_default_privacy_policy_content' filter.

Props claudiu.
Merges [42985] to the 4.9 branch.
See #43620.

#70 @SergeyBiryukov
6 years ago

In 43113:

Privacy: make the sections in the suggested privacy policy text postbox foldable. Add Read More/Read Less buttons. Fix copying of the suggested text by pressing the button.

Props melchoyce, xkon, azaozz.
Merges [42992] to the 4.9 branch.
See #43620.

#71 @SergeyBiryukov
6 years ago

In 43114:

Make the string WordPress translatable.

Props mnelson4.
Merges [42999] to the 4.9 branch.
See #43620.

#72 @SergeyBiryukov
6 years ago

In 43115:

Privacy: add better docs for wp_add_privacy_policy_content() and WP_Privacy_Policy_Content::add().

Props azaozz.
Merges [43003] to the 4.9 branch.
See #43620.

This ticket was mentioned in Slack in #gdpr-compliance by xkon. View the logs.


6 years ago

This ticket was mentioned in Slack in #gdpr-compliance by desrosj. View the logs.


6 years ago

This ticket was mentioned in Slack in #gdpr-compliance by allendav. View the logs.


6 years ago

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


6 years ago

This ticket was mentioned in Slack in #gdpr-compliance by xkon. View the logs.


6 years ago

#78 @xkon
6 years ago

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

#79 @desrosj
6 years ago

  • Component changed from General to Privacy

Moving to the new Privacy component.

Note: See TracTickets for help on using tickets.