[wp-trac] [WordPress Trac] #43715: Add Privacy Policy link to bundled theme footers

[WordPress Trac]

#43715: Add Privacy Policy link to bundled theme footers
-------------------------+-----------------------
 Reporter:  xkon         |       Owner:  xkon
     Type:  enhancement  |      Status:  assigned
 Priority:  normal       |   Milestone:  4.9.6
Component:  General      |     Version:
 Severity:  normal       |  Resolution:
 Keywords:  gdpr         |     Focuses:  ui
-------------------------+-----------------------

Comment (by iandunn):

 Replying to [comment:22 xkon]:
 > [attachment:43715.2.diff] makes a first pass in all bundled themes to
 add a footer link to Privacy Policy page

 That looks good to me at a quick glance. Here's a few suggestions:


 * There's a coding standard guideline about not setting variables inside
 `if ( )` blocks. It seems cleaner to set it at the top of the file:

 {{{
 <?php
 /**
  * The template for displaying the footer
  *
  * Contains the closing of the #content div and all content after.
  *
  * @link https://developer.wordpress.org/themes/basics/template-files
 /#template-partials
  *
  * @package WordPress
  * @subpackage Twenty_Seventeen
  * @since 1.0
  * @version 1.2
  */

 $privacy_policy_url = get_privacy_policy_url();

 ?>
 }}}

 I'm not sure if that's a common convention in Core, though, so maybe be
 more appropriate to define itjust above the `if ( )` block.

 * Rather than `echo`ing HTML, it seems more readable to me to interpolate
 `<?php` tags:

 {{{
 <?php if ( $privacy_policy_url ) : ?>
         <a class="privacy-policy-page-link" href="<?php echo esc_url(
 $privacy_policy_url ); ?>" title="<?php echo esc_attr( 'Privacy Policy',
 'twentyseventeen' ); ?>">
                 <?php _e( 'Privacy Policy', 'twentyseventeen' ); ?>
         </a>
 <?php endif; ?>
 }}}

 That may also not fit with existing Core conventions, though.

 * I think `esc_url()` is better for the URL than `esc_attr()`, because
 it's a more targeted context.
 * Similarly, I think `esc_attr()` is better for the `title` than
 `esc_html()`.
 * Is the `span` necessary? I wonder if it'd be more semantic, and more
 consistent with the existing links in the footers, to just have an `a`
 element with the class?
 * Do we need the `<br>` for back-compat? If not, then it seems more
 semantic to use something like `.privacy-policy-page-link { display: block
 }`.
 * If we keep the `<br>` should we use `<br />` in some of the themes,
 depending on their `doctype`?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/43715#comment:24>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform