Opened 8 years ago
Last modified 7 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: |
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)
Change History (9)
#1
@
7 years ago
- Keywords reporter-feedback needs-patch added
- Milestone changed from Awaiting Review to Future Release
#3
@
7 years ago
- Keywords reporter-feedback removed
Cool, once you have a patch we can milestone for 4.9.
#4
@
7 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
@
7 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.
@dlh good catch. What if the
starter_content
flag for a given setting in the changeset were to be changed fromtrue
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 astarter_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 thestarter_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.