Make WordPress Core

Opened 13 years ago

Closed 8 years ago

Last modified 2 years ago

#17609 closed defect (bug) (fixed)

'View post' link shown even when post type can't be viewed on the front-end

Reported by: scribu's profile scribu Owned by: johnbillion's profile johnbillion
Milestone: 4.4 Priority: normal
Severity: normal Version: 2.9
Component: Posts, Post Types Keywords: has-patch needs-refresh
Focuses: Cc:

Description (last modified by scribu)

I have registered a CPT that's not meant to be displayed on the front-end:

        'public' => false,
        'show_ui' => true,

If I create such a post in the admin area, I still see the 'View post' link after saving and the 'Preview changes' button.

Both send me to the same URL, which gives a 404:

?post_type_name=single-post-slug

Related: #17040

Attachments (10)

17609.diff (4.1 KB) - added by mfields 13 years ago.
17609-new-strings.diff (1.2 KB) - added by mfields 13 years ago.
meta-boxes.php.diff (833 bytes) - added by intoxstudio 12 years ago.
17609-combo.diff (4.6 KB) - added by jmslbam 11 years ago.
class-wp-posts-list-table.patch (1.0 KB) - added by jfarthing84 11 years ago.
17609.2.diff (1.9 KB) - added by theaussiepea 11 years ago.
17609.patch (551 bytes) - added by tyxla 9 years ago.
If the post type is not publicly queryable, display the "Post Updated" / "Page Updated" message instead of the ones with "View" links.
17609.3.diff (5.1 KB) - added by wonderboymusic 9 years ago.
17609.4.diff (5.5 KB) - added by DrewAPicture 9 years ago.
17609.5.diff (4.8 KB) - added by johnbillion 9 years ago.

Download all attachments as: .zip

Change History (78)

#1 @scribu
13 years ago

  • Description modified (diff)

@mfields
13 years ago

#2 @mfields
13 years ago

  • Cc michael@… added
  • Keywords has-patch reporter-feedback added

I verified this while running trunk with my own custom post type. I've put together a patch which works well for me. This solution should work for all post_types. it will only print links to the front-end if the post type is "publicly queryable".

In hindsight, I think that the patch may do too much. I've also added code to dynamically populate the nouns in each message where appropriate. Instead of hardcoding "Post" it will use the singular label from the post_types labels array. This deviates a bit from the scope of this ticket. I thought that while I was in there I might as well address these issues too.

Definitely needs testing.

#3 @dd32
13 years ago

Instead of hardcoding "Post" it will use the singular label from the post_types labels array. This deviates a bit from the scope of this ticket.

It also causes translation problems unfortunately, In quite a lot of languages you can't simply slot a "object label" into a generic string - In English this is no problem, but when you start to deal with feminine/mascular/cultural words, 'Page' and 'Post' require the strings around them to be translated differently.

If a plugin/theme wishes for their Messages to be translated, they should instead use the 'post_updated_messages' filter, as shown in the 2nd half of the example here: Function_Reference/register_post_type#Example

Given the above, The View Post link can already be removed by the plugin since they can modify their strings already.. I like the suggestion of using a placeholder for the "actions" attached, but this would also cause problems for those using the filter and expecting it to be a url (rather than an actual HTML link)

#4 @mfields
13 years ago

Thanks for the detailed overview of some of the concerns with the first patch. Translation is definitely a topic that I can do to learn a lot more about. I have a second patch to offer for consideration (17609-new-strings.diff). This time, I stuck only to the topic of the ticket. Instead of modifying strings, I have introduced similar strings that will get used only when a custom post_type has not provided messages via the 'post_updated_messages' filter and the post_type is not publicly queryable.

#5 @dgwyer
13 years ago

Having the same problem. I can change the 'View Post', and 'Updated Post' phrases via the 'post_updated_messages' filter.

But the 'Preview' and 'Preview Changes' button in the Publish meta box seems hard coded in /wp-admin/includes/meta-boxes.php on line 51. The only way to remove this currently is via jQuery after the page has loaded?

Wouldn't it be sufficient to just add another filter to the Publish meta box so that you could modify the 'Preview' and 'Preview Change' buttons etc?

#6 @tamlyn
12 years ago

mfields' patch looks good to me. Any chance of getting this fixed in core?

#7 @tamlyn
12 years ago

  • Cc tamlyn added

#9 @scribu
12 years ago

  • Keywords reporter-feedback removed

#10 @nacin
12 years ago

  • Owner set to nacin
  • Status changed from new to reviewing

#11 @intoxstudio
12 years ago

I thought I'd searched for related tickets before I created one, but obviously I didn't do it well. Sorry about that.

I've had some thoughts since I made my ticket and read this one: Is there a reason why some post-related labels are stored in the post object while others aren't? (current labels vs. updated message labels)

Also, while the messages can be overwritten with a filter, the preview button cannot be removed without fiddling with some css (or js as already mentioned) which is not a proper solution I think.

#12 @markoheijnen
12 years ago

  • Cc marko@… added

Any changes this get in 3.4? I'm fine with the patch of intoxstudio only since that is the real problem.
The messages can be changed with the filter. Something what everyone probably should do.

#13 @scribu
12 years ago

  • Milestone changed from Future Release to 3.4

#14 @husobj
12 years ago

  • Cc ben@… added

#15 @nacin
12 years ago

  • Keywords commit added

When the post is published and there is no draft/pending button, there is some extra space in the publish box: http://cl.ly/161h1K29331V3I3Q1T21

This space is made due to the hidden draft-ajax-loading spinner. It uses visibility: hidden, as normally it should maintain its space.

What we'd need to do is calculate for when there is no button and at that point choose not to show the spinner either.

Regardless, the existing patch is better than what we have now.

#16 @nacin
12 years ago

In [20667]:

Don't show the 'Preview Changes' button for a non-public post type. props intoxstudio, see #17609.

#17 @nacin
12 years ago

  • Milestone changed from 3.4 to Future Release

#18 @sc0ttkclark
11 years ago

  • Cc lol@… added

I was a sad panda when I saw this issue come up in my testing, sad panda.

#19 @SergeyBiryukov
11 years ago

#22398 was marked as a duplicate.

@jmslbam
11 years ago

#20 @jmslbam
11 years ago

  • Cc info@… added

So I took the liberty of merging the two diffs of mfields so the current translations that are using the filter and expecting it to be a url (rather than an actual HTML link) like dd32 pointed out.

I really takes out the hassle of defining custom update messages just to have it say 'Book published'

#21 @scribu
11 years ago

@jmslbam: Please re-read dd32's comment: http://core.trac.wordpress.org/ticket/17609#comment:3

In quite a lot of languages you can't simply slot a "object label" into a generic string - In English this is no problem, but when you start to deal with feminine/mascular/cultural words, 'Page' and 'Post' require the strings around them to be translated differently.

Last edited 11 years ago by scribu (previous) (diff)

#22 @scribu
11 years ago

  • Milestone changed from Future Release to 3.4
  • Resolution set to fixed
  • Status changed from reviewing to closed

Actually, this ticket was fixed with [20667], so get off my lawn and please open a new ticket for dealing with update messages.

#23 @Jesper800
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This is actually not fixed. Viewing the fix, http://core.trac.wordpress.org/changeset/20667, you can see that it only checks for the "public" property of a post type. When a post type is public but not publicly queryable (public true, publicly_queryable false) posts of the post type are not available by default on the frontend, so the link should not be shown, but currently, they are.

A problem with this is that pages are not publicly queryable, as they are handled separately. What would be a good way to actually check whether a post is available on the frontend?

Last edited 11 years ago by Jesper800 (previous) (diff)

#24 @Jesper800
11 years ago

  • Cc jeve0@… added

#25 @SergeyBiryukov
11 years ago

  • Keywords commit removed
  • Milestone changed from 3.4 to Awaiting Review

#26 @johnbillion
11 years ago

  • Cc johnbillion added

#27 @DrewAPicture
11 years ago

#23814 was marked as a duplicate.

#28 @netweblogic
11 years ago

  • Cc msykes@… added

#29 @Daedalon
11 years ago

  • Cc daedalon@… added

#30 @alex-ye
11 years ago

  • Cc nashwan.doaqan@… added

#31 @MikeHansenMe
11 years ago

  • Cc mdhansen@… added

#32 @toscho
11 years ago

  • Cc info@… added

#33 @jfarthing84
11 years ago

  • Cc jeff@… added

Maybe I'm missing something, but this is a pretty simply fix of just checking publicly_queryable instead of public.

This also applies to a ticket I was going to create, but is actually closely related with this. That is, a View link is shown on the edit.php page even when a post type is not publicly_queryable, because yet again, the code checks public instead.

#34 @jfarthing84
11 years ago

For the record, this can be worked around by setting public to false, and then explicity setting show_ui to true, as well as all of the parameters that inherit from public. However, this shouldn't have to be done.

#35 @bungeshea
11 years ago

  • Cc info@… added

#37 @theaussiepea
11 years ago

Confirming that

class-wp-posts-list-table.patch

Removes the "View" option for each post in wp-posts-lists for custom posts. However, the permalink editing for the edit single post page (wp-admin/post.php) is still visible.

So I added to the patch so permalink editing does not appear in the edit single post page for custom post types that have 'publicly_queryable' set to false.

It seems to make sense to use 'publicly_queryable' instead of 'public' since 'publicly_queryable', in our testing, is what controls whether you can see the post at the specified permalink url. (After talking to Nacin, this is more complicated, so more work is needed)

Props to leogermani and ancawonka for their help!

#38 @johnbillion
10 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Version set to 2.9

#39 @johnbillion
10 years ago

#29375 was marked as a duplicate.

#40 @johnbillion
10 years ago

It seems that the publicly_queryable argument has an identity crisis. What purpose does it serve to set this to a value which is different to that of public? Setting publicly_queryable to the opposite of public always results in broken behaviour. Example:

register_post_type( 'test', array(
	'show_ui'            => true,
	'public'             => true,
	'publicly_queryable' => false,
) );

The above code results in a post type with 'View' links throughout the admin area which, when clicked, result in a 404 because the post type is not publicly queryable.

register_post_type( 'test', array(
	'show_ui'            => true,
	'public'             => false,
	'publicly_queryable' => true,
) );

Conversely, the above code results in a post type without any 'View' links in the admin area, but a post type which is publicly queryable and which can be linked to.

What is the use case for ever setting publicly_queryable to the opposite of public?

In addition, the publicly_queryable argument for the Page post type is set to false, yet Pages are indeed publicly queryable. Has the purpose of this argument become confused?

Last edited 10 years ago by johnbillion (previous) (diff)

#41 @husobj
10 years ago

Just to throw this use-case into the mix as it's something I already do... just something to consider.

I have a custom post type which is not publicly queryable.
The post type is used to automatically add content to a 'pages' specified by the parent_id.

So I use link filters to customise the URLs for my non-public post type to link to the public parent page, which means my preview links link to the 'page' where my custom post type is used.

Would be good if whichever method is used to fix this includes a filter so that it is still possible to include a preview link even if publicly_queryable is set to false.

This ticket was mentioned in Slack in #accessibility by sergeybiryukov. View the logs.


9 years ago

#43 @SergeyBiryukov
9 years ago

#32271 was marked as a duplicate.

#44 @johnbillion
9 years ago

#32379 was marked as a duplicate.

@tyxla
9 years ago

If the post type is not publicly queryable, display the "Post Updated" / "Page Updated" message instead of the ones with "View" links.

#45 @tyxla
9 years ago

I think this one would have the least impact if we do the change I suggested in https://core.trac.wordpress.org/attachment/ticket/17609/17609.patch

In this patch, if the post/page is not publicly queryable instead of the messages that contain links, we use the simple Post Updated / Page Updated message (without "View" links).

#46 @wonderboymusic
9 years ago

  • Milestone changed from Future Release to 4.4

#47 @wonderboymusic
9 years ago

  • Owner nacin deleted
  • Status changed from reopened to assigned

I think 17609.3.diff handles this well and rewrites the way messages are composed (an improvement IMHO)

publicly_queryable means something different for _builtin types. To determine if a post type is viewable, we should do this check:

$post_type->publicly_queryable || ( $post_type->_builtin && $post_type->public )

This handles every case well in my testing. To test, make a custom post type like so:

add_action( 'init', function () {
	register_post_type( 'tacos', [
		'public' => true,
		'publicly_queryable' => false,
		'labels' => [
			'name' => 'Tacos'
		]
	] );
} );

@DrewAPicture
9 years ago

#48 @DrewAPicture
9 years ago

17609.4.diff adds some sanity in formatting to that ternary/sprintf block, and adds complete docs for is_post_type_viewable().

#49 @wonderboymusic
9 years ago

In 33666:

Custom Post Types:

  • Introduce is_post_type_viewable( $post_type_object )
  • Separate the HTML bits from the translatable bits in the post messages array in edit-form-advanced.php
  • Don't show certain UI pieces when a post is not viewable on the front end

When a custom post type item is not viewable on the front end, we don't want to show links to View it (on the front end) all over the admin. We also want to hide the Preview link, et al. We also want our admin messages to not contain said links.

Custom post types with public_queryable set to false are not viewable on the front end.
'page' is viewable on the front end, but 'page' is a _builtin type, and public_queryable is set to false when it is registered - see WP::parse_request() for when public_queryable gets used.

This is confusing, but also somewhat straightforward: to determine if a post type is viewable on the front end, we can check one way for _builtin => true and another way for _builtin => false:

$post_type->publicly_queryable || ( $post_type->_builtin && $post_type->public )

If a post type is publicly_queryable, it's viewable. If that value is false, it is viewable if it's a _builtin type that is also public.

I am in search of edge cases, so this shall land.

Props wonderboymusic, DrewAPicture.
See #17609.

#50 @wonderboymusic
9 years ago

In 33672:

After [33666], fix broken sprintf cruff.

See #17609.

#51 @wonderboymusic
9 years ago

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

#52 follow-up: @bradyvercher
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I have a plugin that allows for managing archive titles, descriptions and permalink slugs using a CPT to house that data in the background. The background CPT shouldn't be publicly queryable, but it still uses the "edit slug box" for managing permalinks. This isn't possible after this line was added in [33666].

Here's the plugin and registration args I'm using for testing. This is probably an edge case, but it would be nice to be able to continue supporting this functionality.

#53 follow-up: @johnbillion
9 years ago

  • Keywords needs-patch added; has-patch removed

The concatenating of $view_post_html and $preview_link_html onto the end of the confirmation messages means they're not properly localisable. This will need to be corrected.

#54 @johnbillion
9 years ago

  • Owner set to johnbillion
  • Status changed from reopened to accepted

@johnbillion
9 years ago

#55 @johnbillion
9 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

17609.5.diff reverts the post updated message concatenation introduced in [33666] and replaces it with conditional logic which builds the complete post updated messages based on whether the post type is viewable or not.

/cc @dd32 @ocean90

#56 in reply to: ↑ 52 ; follow-ups: @johnbillion
9 years ago

Replying to bradyvercher:

I have a plugin that allows for managing archive titles, descriptions and permalink slugs using a CPT to house that data in the background. The background CPT shouldn't be publicly queryable, but it still uses the "edit slug box" for managing permalinks. This isn't possible after this line was added in [33666].

The post slug can still be edited by enabling the 'Slug' meta box in Screen Options. This might even be a preferable UI for your plugin because it doesn't suggest that the slug is used in a publicly accessible permalink. Let me know what you think.

#57 in reply to: ↑ 56 @helen
9 years ago

Replying to johnbillion:

The post slug can still be edited by enabling the 'Slug' meta box in Screen Options. This might even be a preferable UI for your plugin because it doesn't suggest that the slug is used in a publicly accessible permalink. Let me know what you think.

This reminded me that I had envisioned the slug field being in that position at all times, whether for a permalink or a slug being used in other ways, and then getting rid of the metabox as default UI. See #18523.

#58 follow-up: @wonderboymusic
9 years ago

@johnbillion - gonna defer to this ticket for these translation strings: #31858

@helen do you want to keep this ticket open for the Slug or can it be closed?

#59 in reply to: ↑ 56 @bradyvercher
9 years ago

Replying to johnbillion:

The post slug can still be edited by enabling the 'Slug' meta box in Screen Options. This might even be a preferable UI for your plugin because it doesn't suggest that the slug is used in a publicly accessible permalink. Let me know what you think.

It actually is a publicly accessible permalink since it maps to the CPT archive, which is why the existing UI works well. Would it be possible to get a filter to enable that section manually instead of cutting off access?

It's not the end of the world if the slug meta box has to be used, but most users don't know it even exists or what it's used for.

#60 in reply to: ↑ 58 @helen
9 years ago

Replying to wonderboymusic:

@helen do you want to keep this ticket open for the Slug or can it be closed?

Using the other ticket for that. It will mean that that line gets removed, by the way. Let's leave this open for now though because I don't like that it's using if: endif; and if we don't end up going with having a consolidated permalink/slug editing UI for some reason, we need to fix that and possibly introduce a filter or similar, per @bradyvercher.

#61 in reply to: ↑ 53 ; follow-up: @SergeyBiryukov
9 years ago

Replying to johnbillion:

The concatenating of $view_post_html and $preview_link_html onto the end of the confirmation messages means they're not properly localisable. This will need to be corrected.

[33666] actually looks good to me as is. These strings are separate sentences and can be translated separately. RTL should not be an issue either, text direction should take care of that.

#62 in reply to: ↑ 61 @ocean90
9 years ago

  • Keywords needs-refresh added; dev-feedback removed

Replying to SergeyBiryukov:

[33666] actually looks good to me as is. These strings are separate sentences and can be translated separately. RTL should not be an issue either, text direction should take care of that.

That's correct.

#63 @ramiy
9 years ago

I see one issue:

[33666] fixes translation strings for $messages['post'], but not $messages['page'].

#64 @ramiy
9 years ago

I have added a new patch to #31858 fixing the translation strings issue I mentioned above.

#65 @johnbillion
8 years ago

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

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


8 years ago

This ticket was mentioned in Slack in #mobile by sergey. View the logs.


7 years ago

This ticket was mentioned in Slack in #polyglots by sergey. View the logs.


2 years ago

Note: See TracTickets for help on using tickets.