Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#46831 closed enhancement (fixed)

Warn users when Privacy Policy page is set as 'Homepage' or 'Posts page'

Reported by: garrett-eclipse's profile garrett-eclipse Owned by: garrett-eclipse's profile garrett-eclipse
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch needs-testing
Focuses: ui, privacy Cc:

Description

Hello,

If you set the 'Homepage' and 'Posts page' to the same page you get a warning;
"Warning: these pages should not be the same!"

It would be nice if a similar warning would indicate if the Privacy Policy page was accidentally set as either. Maybe a message along the lines of;
"Warning: these pages should not be the same as your Privacy Policy page!"

Thanks

Attachments (5)

Screen Shot 2019-04-08 at 1.43.15 AM.png (97.3 KB) - added by garrett-eclipse 5 years ago.
The warning when same page is used for 'Homepage' and 'Posts page'
46831.diff (863 bytes) - added by subrataemfluence 5 years ago.
Screen Shot 2019-04-08 at 11.38.39 PM.png (104.7 KB) - added by garrett-eclipse 5 years ago.
Successfully testing the warning
46831.2.diff (1.3 KB) - added by garrett-eclipse 5 years ago.
Minor update to @subrataemfluence's lead
46831.simple.diff (1.1 KB) - added by garrett-eclipse 5 years ago.
Simplified version

Download all attachments as: .zip

Change History (16)

@garrett-eclipse
5 years ago

The warning when same page is used for 'Homepage' and 'Posts page'

#1 @mukesh27
5 years ago

  • Keywords needs-patch added

#2 follow-up: @subrataemfluence
5 years ago

  • Keywords reporter-feedback added

It would be a good addition since WordPress is looking at Privacy page a bit differently. However, the problem is the post_type being used by this page is page.

What I have understood so far is if we can incorporate an additional native post_type called Privacy and that gets associated with the Privacy page created from Settings -> Privacy menu, it would be easier to handle things associated with this page.

In the context of this ticket we can then put a check like:

<?php if ( 'privacy' == get_option( 'show_on_front' ) || 'privacy' == get_option( 'page_for_posts' ) ) : ?>
   <div id="front-page-warning" class="error inline"><p><?php _e( '<strong>Warning:</strong> Warning: these pages should not be the same as your Privacy Policy page!' ); ?></p>
   </div>
<?php endif; ?>

Although I am not sure how feasible it would be to create another post_type!

However, one can create his own Privacy page without using Settings->Privacy menu and that is always treated as page post_type.

In this scenario, the above approach will not work. Even an additional post_type privacy is introduced (?), the problem will still remain for custom Privacy policy pages.

Last edited 5 years ago by subrataemfluence (previous) (diff)

#3 in reply to: ↑ 2 @garrett-eclipse
5 years ago

  • Keywords reporter-feedback removed

Replying to subrataemfluence:

It would be a good addition since WordPress is looking at Privacy page a bit differently. However, the problem is the post_type being used by this page is page.

What I have understood so far is if we can incorporate an additional native post_type called Privacy and that gets associated with the Privacy page created from Settings -> Privacy menu, it would be easier to handle things associated with this page.

In the context of this ticket we can then put a check like:

<?php if ( 'privacy' == get_option( 'show_on_front' ) || 'privacy' == get_option( 'page_for_posts' ) ) : ?>
   <div id="front-page-warning" class="error inline"><p><?php _e( '<strong>Warning:</strong> Warning: these pages should not be the same as your Privacy Policy page!' ); ?></p>
   </div>
<?php endif; ?>

Although I am not sure how feasible it would be to create another post_type!

However, one can create his own Privacy page without using Settings->Privacy menu and that is always treated as page post_type.

In this scenario, the above approach will not work. Even an additional post_type privacy is introduced (?), the problem will still remain for custom Privacy policy pages.

Thanks @subrataemfluence I appreciate the feedback. However, I would strongly suggest against a CPT for privacy pages as it would unnecessarily complicate the content creation and association processes. Currently, admins can create a page or use an existing page to associate it as the Privacy Policy page, forcing a CPT adds additional load and would require existing pages be converted to the new post type. As well this change would almost hide the Privacy Policy pages on the Menu Editor as Pages are default expanded for selection so if a new CPT was introduced it would become hidden like posts and custom link settings. As well as a CPT it would additional load to site owners and theme developers as they'd need to provide default template for it whereas now the page.php is being used, and any styling done for pages would need to be replicated.
IMHO using a CPT for privacy pages unnecessarily complicates things and forces replication of existing functionality already available for pages. As well with all the helpers being introduced in #44005 in 5.2 developers can more easily customize their privacy page and menu items separate from their standard pages if they so desire.

Sidenote: The show_on_front and page_for_posts options return page IDs so would need to compare those against the wp_page_for_privacy_policy page ID as follows;

<?php
$privacy_policy_page = get_option( 'wp_page_for_privacy_policy' );
if ( $privacy_policy_page == get_option( 'show_on_front' ) || $privacy_policy_page == get_option( 'page_for_posts' ) ) : ?>

*The example you provided would be unnecessary if privacy was a CPT as the dropdown only calls pages so no check would be needed.

I appreciate the feedback and discussion but feel we should avoid complicating things with a drastic change such as switching to a CPT.

All the best

#4 @subrataemfluence
5 years ago

@garrett-eclipse I perfectly understand what you said. Thank you for explaining it so nicely. I was not in super favor of adding another CPT, but was thinking if that could be an option so only proposed! :)

And yes, the ID is available and we can easily use it to determine like you mentioned.

Version 0, edited 5 years ago by subrataemfluence (next)

#5 @subrataemfluence
5 years ago

  • Keywords has-patch added; needs-patch removed

Hi @garrett-eclipse please let me know if the patch works.

By the way, get_option( 'page_on_front' ) returns the ID while get_option( 'show_on_front' ) returns the CPT type. I know it was nothing but a typo :)

@garrett-eclipse
5 years ago

Successfully testing the warning

@garrett-eclipse
5 years ago

Minor update to @subrataemfluence's lead

@garrett-eclipse
5 years ago

Simplified version

#6 @garrett-eclipse
5 years ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 5.3
  • Owner set to garrett-eclipse
  • Status changed from new to assigned

Thanks @subrataemfluence I appreciate the patch and like the direction, you're on the ball.

Testing went nicely, screen attached.

I made some minor tweaks in 46831.2.diff making the id unique and adding some spacing, as well as adding the @since to 5.3 as this is an enhancement and 5.2 is already in beta.

While looking at the original front-page warning I was wondering if we should just do a simplified version to match the front-page-warning, so uploaded 46831.simple.diff as an alternate.

I appreciate you moving this forward, all the best

#7 @SergeyBiryukov
5 years ago

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

In 45766:

Administration: Show a warning in Reading Settings when a Privacy Policy page is accidentally set as a Homepage or Posts page.

Props garrett-eclipse, subrataemfluence.
Fixes #46831.

#8 follow-up: @alFrame
4 years ago

This is showing also when there's no page selected as post page. Very confusing...

#9 in reply to: ↑ 8 ; follow-up: @garrett-eclipse
4 years ago

Replying to alFrame:

This is showing also when there's no page selected as post page. Very confusing...

Hi @alFrame I assume you're seeing this when you set the homepage to the privacy page which is the expected behaviour even if there's nothing set for the posts page. The verbiage used is meant to be generic to cover either case;
"Warning: these pages should not be the same as your Privacy Policy page!"

Would alternate verbiage reduce the confusion? Something like;
"Warning: Neither of these pages should be the same as your Privacy Policy page!"

Happy to discuss further, but as this ticket is already committed and live in 5.3 I suggest you open a new ticket;
https://core.trac.wordpress.org/newticket?component=Administration&description=This%20is%20a%20follow-up%20to%20%2346831.

Thank you

#10 in reply to: ↑ 9 @alFrame
4 years ago

Replying to garrett-eclipse:

Hi @alFrame I assume you're seeing this when you set the homepage to the privacy page which is the expected behaviour even if there's nothing set for the posts page. The verbiage used is meant to be generic to cover either case;
"Warning: these pages should not be the same as your Privacy Policy page!"

This also shows when I set the home page to my home page which is not the privacy policy page. It only changes if I set something as the post page. Unless the cookie notice plugin is sniffed by the code behind this and thinks that my front page is my privacy policy page...

I will know more when I start updating all my clients sites. This might also be related to the theme builder I am using or other factors.

Would alternate verbiage reduce the confusion? Something like;
"Warning: Neither of these pages should be the same as your Privacy Policy page!"

That would be awesome! So there would be three stages:
Basically no warning at all as long as the user does not set either both to the same or any of them to the privacy policy page.
And then accordingly: "Warning: These pages should not be the same!"
or: "Warning: Neither of these pages should be the same as your Privacy Policy page!"

Happy to discuss further, but as this ticket is already committed and live in 5.3 I suggest you open a new ticket;
https://core.trac.wordpress.org/newticket?component=Administration&description=This%20is%20a%20follow-up%20to%20%2346831.

I will create a new ticket after I know better if it's setup related or a generic appearance.

Thank you

Thank you!

#11 @garrett-eclipse
4 years ago

Thanks @alFrame feel free to ping me (@garrett-eclipse) when you create a new ticket and I'll be happy to work through this with you. All the best

Note: See TracTickets for help on using tickets.