WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 2 years ago

#40146 new defect (bug)

Starter content can be silently published with other themes

Reported by: dlh Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.7
Component: Customize Keywords: needs-patch
Focuses: Cc:
PR Number:

Description

To replicate on a fresh install, open the Customizer with Twenty Seventeen active, preview a different theme, then "Save & Activate" the new theme. All of the Twenty Seventeen starter content will be published along with the new theme.

The potentially confusing part of this behavior is that starter content might or might not have been in the preview, depending on the new theme.

For example, in Twenty Eleven, some of the Twenty Seventeen starter widgets are visible in the preview, but the menu items aren't.

I'm not quite sure what the preferred behavior would be in this scenario, assuming it's not the current behavior. The attached patch would have WP_Customize_Manager::unsanitized_post_values() skip any changeset values flagged as starter content when the current theme doesn't support it, but that might not be desirable.

Attachments (2)

40146.diff (903 bytes) - added by dlh 3 years ago.
40146.2.diff (10.0 KB) - added by dlh 2 years ago.

Download all attachments as: .zip

Change History (9)

@dlh
3 years ago

#1 @westonruter
3 years ago

  • Keywords reporter-feedback needs-patch added
  • Milestone changed from Awaiting Review to Future Release

@dlh good catch. What if the starter_content flag for a given setting in the changeset were to be changed from true to be the slug of the theme for which it is starter content? Then when switching to preview another theme, the reading of the values from the changeset could skip any that have a starter_content that doesn't match the current theme. If the user switches back to the initial theme again, the starter content from that theme could then be re-applied if the starter_content only if the flag is still present, as when the flag is removed this is an indication that the user modified the value and it is no longer starter content.

#2 @dlh
2 years ago

I like the idea of using the theme slug instead of true. I can work on a patch.

#3 @westonruter
2 years ago

  • Keywords reporter-feedback removed

Cool, once you have a patch we can milestone for 4.9.

@dlh
2 years ago

#4 @dlh
2 years ago

I'm not sure that using the theme slug is going to be sufficient, or at least not in all cases. The primary reason is that some values in the changeset are shared among themes, such as nav_menus_created_posts.

40146.2.diff is attempt to make the starter_content flag into an array with 'themeslug' => true, but I suspect the direction of the patch as written is a dead end. There are a lot of self-inflicted annoyances, and themes can still overwrite other themes' settings.

I'm planning to start fresh, having learned a few lessons from the patch. If it inspires any other ideas, I'm definitely open to them.

#5 @westonruter
2 years ago

Yeah, nav_menus_created_posts would be particularly difficult because it contains post IDs that may have been created for starter content _or_ as part of manually adding pages for nav menu items or the static homepage/posts-page. Are there other cases of settings that get silently published? If it is just a special case for nav_menus_created_posts then one option could be to check if there are still nav menu items present that reference these IDs. If there are no reference counts for these IDs, then they could be skipped from being published.

Something else is that when a post gets created as starter content and added to nav_menus_created_posts, each post could get its own dedicated postmeta array of the themes that it is starter content for. For example, if theme A and theme B both have a static front/homepage created, if you preview A the auto-draft will be created and it could get a added to this postmeta. Then when switching to preview theme B, the importer logic should be skipping to insert another auto-draft for this homepage since it already exists in nav_menus_created_posts and would be re-used, but when it is re-used then it could get the theme b added to this postmeta. In that way, this postmeta could serve as another way to do reference counting.

This ticket was mentioned in Slack in #core-customize by dlh. View the logs.


2 years ago

#7 @westonruter
2 years ago

In 42107:

Customize: Prevent re-importing starter content when changeset is saved as draft or scheduled.

Themes cannot currently be switched in Customizer after changeset is saved anyway.

Props dlh, westonruter.
See #40146, #42411, #42126.
Fixes #42395.

Note: See TracTickets for help on using tickets.