Opened 4 years ago

Last modified 3 months ago

#8592 reopened enhancement

Private Pages not listed in the Parent dropdown

Reported by: mtdewvirus Owned by: nacin
Priority: normal Milestone: Future Release
Component: Administration Version: 2.7
Severity: major Keywords: has-patch commit 3.5-early
Cc: vinceger, jeremyduffy, Kanuck54, mcrea1248, rwbbonn, Yves, sillybean, deoli, jcollier, WordPress@…, bsamrajni, Victor, Delta, cnorris23+wordpress@…, mikemcmurray, wikichaves, inge12, ben@…, davecpage, kmklr72, mattheweppelsheimer, mike@…, chappellind@…, seancojr, knut@…, manuel@…, miss_jwo, robwil0

Description

Private pages are not available as a choice in the Parent dropdown of the Attributes module.

You should be able to create a hierarchy of private pages if you want to.

Tested with r10194.

Attachments (17)

post.php.diff (713 bytes) - added by mtekk 4 years ago.
8592-post.php.diff (974 bytes) - added by mtekk 4 years ago.
8592-post.php.2.diff (1.2 KB) - added by mtekk 4 years ago.
This has a fix so that Authors, Contributors, and Subscribers don't cause an invalid query to be made
8592-post.php.3.diff (1.0 KB) - added by mtekk 4 years ago.
uses edit_private_pages privlige instead of read_private_pages
post.php (120.9 KB) - added by bsamrajni 3 years ago.
8592-post.php.3RC1.diff (1022 bytes) - added by sillybean 3 years ago.
This patch seems to work around the problem in 3.0 RC 1.
post-r16235.diff (818 bytes) - added by sillybean 3 years ago.
Refreshing patch
8592-post.php.4.diff (837 bytes) - added by kawauso 2 years ago.
Refresh against 3.1-RC2-17315 and don't escape a static string
8592.diff (2.7 KB) - added by nacin 2 years ago.
Not tested beyond confirming it happens to work.
8592.2.diff (3.5 KB) - added by SergeyBiryukov 2 years ago.
8592.3.diff (4.0 KB) - added by SergeyBiryukov 2 years ago.
8592.4.diff (4.0 KB) - added by SergeyBiryukov 2 years ago.
8592-dropdowns.diff (903 bytes) - added by sillybean 2 years ago.
Adds the page's status in admin dropdown menu
8592.patch (5.2 KB) - added by johnjamesjacoby 2 years ago.
Refresh + combination of patches
meta-boxes.diff (1023 bytes) - added by davecpage 22 months ago.
Allow filtering the args for parent dropdown on Page Attributes metabox
8592.2.patch (4.5 KB) - added by SergeyBiryukov 22 months ago.
8592.3.patch (3.3 KB) - added by SergeyBiryukov 13 months ago.

Download all attachments as: .zip

Change History (167)

comment:1 in reply to: ↑ description   bethallen4 years ago

Replying to mtdewvirus:

Private pages are not available as a choice in the Parent dropdown of the Attributes module.

You should be able to create a hierarchy of private pages if you want to.

Tested with r10194.

Also, pending pages are not shown in the drop down either. I use these to set up a theme for a customer to be able to configure their own child pages. Now it is broken!

  • Component changed from General to Administration
  • Keywords child added
  • Priority changed from normal to high
  • Severity changed from normal to major

This is a bug that really does need to be fixed. I presume the problem is with the parent_dropdown function in template.php, but I don't understand the code there.

parent_dropdown is not the function responsible for that particular dropdown.

The one you're looking for is wp_dropdown_pages, which is defined in wp-includes/post-template.php. That is just a wrapper for get_pages(), however, which is defined in /wp-includes/post.php.

That function should probably include logic to display private pages for admins and, ideally, running the query through a pages_where filter similar to the one used for posts.

Is there a solution already for this problem, or is it fixed in the newest beta?

comment:5   DD324 years ago

Is there a solution already for this problem, or is it fixed in the newest beta?

No. When the functionality is offered in the latest nightly, this ticket will be closed.

If you want to be notified of anything related to this, you can check the CC button.

  • Cc vinceger added

Thanks DD32 for the feedback, I do hope a solution comes fast because I'm stuck here. Ok CC button I will do this. Thanks again !

  • Keywords needs-patch added; parent pages child removed

mtekk4 years ago

  • Keywords has-patch added; needs-patch removed

I took a quick look at this, try this patch out. Not sure if it is kosher to call is_admin() within post.php, but this seems to work.

Thinking more about this, maybe we should add an option called page_types, which would default to contain just 'publish', but could be a comma delimited list. Then in wp_dropdown_pages we can tell it to include 'publish' and 'private' pages.

  • Keywords dev-feedback added

I suppose it'd probably be a good idea to be able to let drafts be parents as well. Think we need some dev feedback on this.

  • Cc jeremyduffy added

Any progress on this? This is kind of a big issue that's been festering for a while. I've left a lot of my private pages alone because I don't want to break their hierarchy, but I can't last much longer.

jeremyduffy, you can try the patch that I attached 5 days ago, it should work. If you could test it and verify that it works for you, then we can add the 'tested' tag to the keywords. I just think we should get some dev feedback as there may be a better way of implementing it.

mtekk4 years ago

ryan, ok I modified that bit to be more like the linked code. See new patch " 8592-post.php.diff"

mtekk,

Next problem after your modification:

  • Now all my child pages of a private Parent page will show up in the sidebar, before all child pages of a private parent page would NOT show up in the sidebar?

Any idea how to solve this ?

Pages or vissible but the drop-down menu (admin) doesnt look like it shoud be

That is only when you are logged in (and have permissions to see them), if you log off you will not see them. Users that are not logged in, or do not have the permissions to see private pages will not see them.

mtekk,

I checked with another login.

With this login the sidebar is totaly empty ??

I'm tracking that problem down right now, will have patch soon.

mtekk4 years ago

This has a fix so that Authors, Contributors, and Subscribers don't cause an invalid query to be made

comment:20 follow-ups: ↓ 21 ↓ 73   mtekk4 years ago

Yeah, I forgot to add $user_ID as a global at the beginning of the function.

comment:21 in reply to: ↑ 20 ; follow-up: ↓ 22   vinceger4 years ago

Replying to mtekk:

Yeah, I forgot to add $user_ID as a global at the beginning of the function.

mtekk,

You're The Man !

Is there any possibility doing the same for admin? If I want to change now I have to scroll for ages. Before (older version) they didn't pop up also)

Maybe you can have a look at the dashboard also, notice the scroll down menu-bar, the bar goes out of the frame.

For the rest, thank you very much I really appreciate this ! For now I don't see any problems.

comment:22 in reply to: ↑ 21 ; follow-up: ↓ 23   mtekk4 years ago

Replying to vinceger:

Replying to mtekk:

Yeah, I forgot to add $user_ID as a global at the beginning of the function.

mtekk,

You're The Man !

Is there any possibility doing the same for admin? If I want to change now I have to scroll for ages. Before (older version) they didn't pop up also)

Maybe you can have a look at the dashboard also, notice the scroll down menu-bar, the bar goes out of the frame.

For the rest, thank you very much I really appreciate this ! For now I don't see any problems.

I'm not sure I quite understand what you are saying/asking. In the dashboard when editing a page the parent attribute should show all pages now except those that would cause a hierarchical loop (e.g. a page being it's own parent, or a parent being a child of one of it's own children or grandchildren). If you are talking about a styling issue with the dropdown, I think that's already part of separate bug (I remember seeing it recently).

comment:23 in reply to: ↑ 22   vinceger4 years ago

Replying to mtekk:

I'm not sure I quite understand what you are saying/asking. In the dashboard when editing a page the parent attribute should show all pages now except those that would cause a hierarchical loop (e.g. a page being it's own parent, or a parent being a child of one of it's own children or grandchildren). If you are talking about a styling issue with the dropdown, I think that's already part of separate bug (I remember seeing it recently).

Im referring to the blog itself, if I login with permission I can see al pages in the sidebar same problem as before with no permission and you solved already.

I have about 600 pages as child pages and prefer not to see them if logged and reading the blog. (I would only like to see them in the dashboard, to change etc.)

Thank's for the drop-down bug, I will search for it.

  • Keywords dev-feedback removed

-1. patch should remove the where clause on the status when a user can view all private stuff.

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 2.8 to Future Release

also, there is the potential issue when a page is public with a private parent. it seems to imply that private pages should always get listed, regardless of whether the user can view them or not.

moving to Future pending new patch

comment:26 in reply to: ↑ 25 ; follow-up: ↓ 27   mtekk4 years ago

Replying to Denis-de-Bernardy:

-1. patch should remove the where clause on the status when a user can view all private stuff.

See the code Ryan linked to, the patch mimics that behavior. If we should remove the where clause entirely from this area, then the same should probably happen there as well.

Replying to Denis-de-Bernardy:

also, there is the potential issue when a page is public with a private parent. it seems to imply that private pages should always get listed, regardless of whether the user can view them or not.

moving to Future pending new patch

I'll test this tonight, but private pages still should not show up with the patch unless the user has permissions to see them.

comment:27 in reply to: ↑ 26 ; follow-up: ↓ 28   Denis-de-Bernardy4 years ago

Replying to mtekk:

I'll test this tonight, but private pages still should not show up with the patch unless the user has permissions to see them.

Err. They should. Or at least the current parent pages should show. Suppose user A publishes a private page that gets used as a stub. And user B publishes a public page whose parent is the private page.

comment:28 in reply to: ↑ 27 ; follow-ups: ↓ 29 ↓ 30   mtekk4 years ago

Replying to Denis-de-Bernardy:

Replying to mtekk:

I'll test this tonight, but private pages still should not show up with the patch unless the user has permissions to see them.

Err. They should. Or at least the current parent pages should show. Suppose user A publishes a private page that gets used as a stub. And user B publishes a public page whose parent is the private page.

I'm not sure if that is really part of this bug, it's more of a behavior question that is opened by any fix to this bug. All child pages, if "public" should really inherit their status from their parent (otherwise what's the point of having them hierarchical if they do not). With the patch I submitted you can have future pages be parents as well, but again, their children should really not be allowed to be public until the parent is (otherwise you'll get a "chicken and egg paradox"). Neither of these were problems before as you "couldn't" have a non-published page be a parent.

comment:29 in reply to: ↑ 28   bethallen4 years ago

Replying to mtekk:

Replying to Denis-de-Bernardy:

Replying to mtekk:

I'll test this tonight, but private pages still should not show up with the patch unless the user has permissions to see them.

Err. They should. Or at least the current parent pages should show. Suppose user A publishes a private page that gets used as a stub. And user B publishes a public page whose parent is the private page.

I'm not sure if that is really part of this bug, it's more of a behavior question that is opened by any fix to this bug. All child pages, if "public" should really inherit their status from their parent (otherwise what's the point of having them hierarchical if they do not). With the patch I submitted you can have future pages be parents as well, but again, their children should really not be allowed to be public until the parent is (otherwise you'll get a "chicken and egg paradox"). Neither of these were problems before as you "couldn't" have a non-published page be a parent.

-- Here is what has happened to me. I created a website for someone under 2.5 or so. I used private pages to create sets of child pages. For example: Current events is page id 2. I can create a bunch of child pages that are public for current events.

The template I wrote for this site does not show all pages the same way in the sidebar. I can write code, for example, that only shows the children of page id 2 in my sidebar. At least that is how it worked until 2.7 changed it. So the site got hosed as soon as someone edited one of those child pages. They could no longer make the private or draft page the parent as it worked originally.

The philosophy that child pages should inherit parent page status is a valid argument, but the latest version broke the way something worked in the older version.

comment:30 in reply to: ↑ 28 ; follow-up: ↓ 31   Denis-de-Bernardy4 years ago

Replying to mtekk:

With the patch I submitted...

My only concern with it (and I take it the core devs will have the same) has to do with performance. Basically, if a user is admin, he should be able to view it all without any extra db check. Else, you're running a query with scores of OR clauses on the status, which is very slow.

comment:31 in reply to: ↑ 30   mtekk4 years ago

Replying to Denis-de-Bernardy:

Replying to mtekk:

With the patch I submitted...

My only concern with it (and I take it the core devs will have the same) has to do with performance. Basically, if a user is admin, he should be able to view it all without any extra db check. Else, you're running a query with scores of OR clauses on the status, which is very slow.

I agree with the idea. However, when I tried to implement it I think I ran into an issue. It has to do with who we actually want to be able to see what pages in the results from get_pages (in the admin dropdown or elsewhere).

right. and thus, it might make sense to do this instead:

where ( post_status = 'publish' or post_parent = $post->post_parent )

that way, the current behavior is kept around, without the bug you (or I) bumped into.

alternatively, there is a function somewhere that might manages this in the sense that it outputs the correct sql. not entirely sure of its name though. it's likely used in wp-includes/query.php and other template functions.

comment:33 in reply to: ↑ 32   mtekk4 years ago

Replying to Denis-de-Bernardy:

right. and thus, it might make sense to do this instead:

where ( post_status = 'publish' or post_parent = $post->post_parent )

that way, the current behavior is kept around, without the bug you (or I) bumped into.

That would partially fix this bug, however administrators wouldn't be able to make pages children of private pages of editors, or other administrators (not sure if this is a problem or not, however if an admin can change page ownership the should be able to use it as a parent, even if it is private).

From http://codex.wordpress.org/Roles_and_Capabilities it looks like we can give anyone with edit_others_pages capabilities the full page list (ends up being administrators and editors). Seems weird that only admins and editors can create pages. I guess just using a blank would work if we check for the edit_others_pages (or maybe even just the read_private_pages) capability.

I guess the other question is do we want these unpublished pages showing up outside of the dashboard for users with the proper privileges (the answer to this may be in the code from query.php that Ryan pointed to (it's now a few lines above the line he linked to))?

comment:34 in reply to: ↑ 25   mark8barnes4 years ago

Replying to Denis-de-Bernardy:

also, there is the potential issue when a page is public with a private parent. it seems to imply that private pages should always get listed, regardless of whether the user can view them or not.

Public pages used to inherit their parents' privacy until 2.5 beta 1. This changed in 2.5RC1. When I reported the change, I got a wontfix, with the developers saying:

"If user want a page to be private, he should set it to private. Inheriting the "private" status can be tricky and misleading"

But regardless of the inheritance issues, we really do need to be able to set pages as children of hidden/future posts.

I suggest that when a parent is hidden, the dropdown on the child is pre-selected to "private", and you are not allowed to change it to "publish".

Likewise with future pages, the child page should 'inherit' the publish time of it's parent, and you should be allowed to set the time later, but not earlier than the parent.

Stopping the user doing what is illogical seems to be the only way forward here.

comment:35 follow-up: ↓ 36   Kanuck544 years ago

  • Cc Kanuck54 added

Just adding myself to the CC list, as this is a pretty severe regression for me.

I used to create a hierarchy of pages with the uppermost parent kept in draft mode; "publish" the pages one by one as they were completed; and then make the whole thing public by publishing the parent. (I think. I could be remembering wrong.)

Anyway, this is obviously not possible right now. Bit of a pain.

I suggest that when a parent is hidden, the dropdown on the child is pre-selected to "private", and you are not allowed to change it to "publish".

Two cents: the only problem I see with this is that it would then be highly troublesome to make a private page with many private descendants public. Adding a bulk action would reduce the pain here, of course.

comment:36 in reply to: ↑ 35   mtekk4 years ago

Replying to Kanuck54:

I suggest that when a parent is hidden, the dropdown on the child is pre-selected to "private", and you are not allowed to change it to "publish".

Two cents: the only problem I see with this is that it would then be highly troublesome to make a private page with many private descendants public. Adding a bulk action would reduce the pain here, of course.

I agree and that's why I think child pages should actually default to the currently hidden "inherit" option. That way you can launch many pages at once by publishing the parent.

  • Cc mcrea1248 added
  • Milestone changed from Future Release to 2.9
  • Version changed from 2.7 to 2.8

Upgraded to 2.8

  • same problem -

editing 8592-post.php.2.diff from mtekk
OK again . . .

@vincegerg: please upload the amended patch

comment:41 in reply to: ↑ 40   vinceger4 years ago

Replying to Denis-de-Bernardy:

@vincegerg: please upload the amended patch

File already attached by mtekk, see top of page.

  • Keywords has-patch added; needs-patch removed

oh, sorry. I was trying to apply this to wp-includes.

  • Version changed from 2.8 to 2.8.1
  • Version changed from 2.8.1 to 2.8.2
  • Version changed from 2.8.2 to 2.8.4

Hello.

I have confirmed that the patch (v2) works as expected in WordPressMU.

What has to happen to get this code into the next version of WP?

  • Cc rwbbonn added
  • Cc Yves added

Very much interested in the resolution of this bug. This is one more step to a proper CMS like management of content in core.

  • Cc sillybean added

Version 2 of the diff appears to be working as of revision 1265 of trunk. Tested under all user roles.

comment:52 follow-up: ↓ 53   deoli4 years ago

  • Cc deoli added
  • Version changed from 2.8.4 to 2.8.5

Just a quick hello from me. I'm using the patch on WP 2.8.5 and I haven't had any problems up to now. I hope this is still being worked on... or that it will be included in future WP updates, it would make work on my current project much easier. Thanks!

comment:53 in reply to: ↑ 52 ; follow-up: ↓ 54   vinceger4 years ago

Replying to deoli:

Just a quick hello from me. I'm using the patch on WP 2.8.5 and I haven't had any problems up to now. I hope this is still being worked on... or that it will be included in future WP updates, it would make work on my current project much easier. Thanks!

Just updated to 2.8.5 and now it's working without the patch ? Is this problem fixed already in version 2.8.5 ?

comment:54 in reply to: ↑ 53 ; follow-up: ↓ 55   mtekk4 years ago

Replying to vinceger:

Replying to deoli:

Just a quick hello from me. I'm using the patch on WP 2.8.5 and I haven't had any problems up to now. I hope this is still being worked on... or that it will be included in future WP updates, it would make work on my current project much easier. Thanks!

Just updated to 2.8.5 and now it's working without the patch ? Is this problem fixed already in version 2.8.5 ?

Nope, still broken in unpatched versions of 2.8.5. Patch still works with r12135 (haven't synced with SVN trunk for a week or so).

comment:55 in reply to: ↑ 54 ; follow-up: ↓ 56   sillybean4 years ago

Still working as of r12181. I believe this patch also fixes #6939.

comment:56 in reply to: ↑ 55 ; follow-up: ↓ 57   vinceger4 years ago

Replying to sillybean:

Still working as of r12181. I believe this patch also fixes #6939.

You're right still working. (also in 2.8.6)

Only one problem remains - if I log in as admin all the child-pages are also there on my blog, is there any patch for this ?

comment:57 in reply to: ↑ 56 ; follow-up: ↓ 58   mtekk4 years ago

Replying to vinceger:

Replying to sillybean:

Still working as of r12181. I believe this patch also fixes #6939.

You're right still working. (also in 2.8.6)

Only one problem remains - if I log in as admin all the child-pages are also there on my blog, is there any patch for this ?

If you have permissions to see the pages, then they should show up (even if they are private or future).

comment:58 in reply to: ↑ 57   vinceger4 years ago

Replying to mtekk:

Replying to vinceger:

Replying to sillybean:

Still working as of r12181. I believe this patch also fixes #6939.

You're right still working. (also in 2.8.6)

Only one problem remains - if I log in as admin all the child-pages are also there on my blog, is there any patch for this ?

If you have permissions to see the pages, then they should show up (even if they are private or future).

Thank sillybean,

I thought that they only had to show up in the 'dashboard' not when reading the blog if logged in.

Thanks again!

  • Milestone changed from 2.9 to Future Release

Nooooo, don't punt! This has been tested, works great in the latest beta, and fixes two problems at once!

  • Keywords tested added
  • Version changed from 2.8.5 to 2.9

I agree with sillybean. What is the point of asking for community involvement in developing and testing patches if they're then ignored?

  • Keywords needs-patch added; has-patch tested removed

patch potentially introduces loads of bugs:

  • the check on is_admin() seems kind of weird.
  • $author_query is pointless if the author can view private pages and/or edit others' page
  • I'm pretty sure the current parent needs to be passed to the query. (haven't tested, but if a non-privileged author creates a page whose parent is owned by someone else and becomes private or pending, what happens when he edits it?)

comment:64 in reply to: ↑ 63   mtekk4 years ago

Replying to Denis-de-Bernardy:

patch potentially introduces loads of bugs:

  • the check on is_admin() seems kind of weird.
  • $author_query is pointless if the author can view private pages and/or edit others' page
  • I'm pretty sure the current parent needs to be passed to the query. (haven't tested, but if a non-privileged author creates a page whose parent is owned by someone else and becomes private or pending, what happens when he edits it?)
  1. Agree, but we need to make sure in the admin/dashboard, any user capable of writing a page and make it a child of anyone's future, pending, or draft page.
  1. Authors can't create, publish, or edit pages. Only editors can, and AFAIK editors can access anything private (or so says edit_private_pages ability).
  1. Ties in with my thought that the page inheritance model of WordPress needs to be rethought. At the moment, it is not very useful in a single site, multiple user setup.

I'm alright with this getting punted, IFF (if and only if) we get one of the core developers to give some input on fixing the page inheritance model.

After looking at get_pages, it looks like we may be able to place this higher up, where it belongs, which will completely eliminate #2 (and allow us to use read_private_pages again instead of edit_private_pages), but I don't remember where that was.

I'm attaching what I have on my current testbed, it's different than the code in patch 2 (uses edit_private_pages).

mtekk4 years ago

uses edit_private_pages privlige instead of read_private_pages

Thanks for the update mtekk !!!!!

  • Cc jcollier added
  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Future Release to 3.0
  • Keywords needs-patch added; has-patch removed

see also #11697, and I don't like the patch much. it'll slow the query down (#11375). also, even if the author cannot edit a private parent page, that page should remain available as a potential parent. in particular when it already is the parent.

comment:70 in reply to: ↑ 69   mtekk3 years ago

Replying to Denis-de-Bernardy:

see also #11697, and I don't like the patch much. it'll slow the query down (#11375). also, even if the author cannot edit a private parent page, that page should remain available as a potential parent. in particular when it already is the parent.

The only thing I'm going to say on #11697 is I feel it is a way to make this issue go away, along with others, with out actually fixing them (you even admitted to it in the comments on #11697).

Anyways, the proper permission level for this problem does not exist at this time (AFAIK). Implemented as is through the patch, most setups would only notice the proper behavior, only editors and admins can access, create, or modify pages. However, in setups with more user levels this could be a problem. So it is not a complete solution.

  • Cc WordPress@… added

See also #4711.

comment:73 in reply to: ↑ 20   sillybean3 years ago

mtekk, I've worked your diff2 into a <a href="http://wordpress.org/extend/plugins/private-suite/">plugin</a> of mine that had a lot of other privacy-related features. Your later patch has better logic, but this one fixes the front end and the back end all at once. My code is clunky as hell, but I desperately needed to stop patching core and move this into a plugin.

  • Keywords bug-hunt added
  • Keywords featured added; bug-hunt removed
  • Owner changed from anonymous to bsamrajni
  • Status changed from new to assigned
  • Cc bsamrajni added
  • Keywords has-patch added; needs-patch removed

I have tried fixing this bug.
Now I'm able to get drafts ang private pages in the dropdown.
All I did was a change in a query in post.php
$query = "SELECT * FROM $wpdb->posts $join WHERE (post_type = 'page' AND (post_status = 'publish' OR post_status = 'draft')) $where ";

comment:78 follow-up: ↓ 80   vinceger3 years ago

  • Version changed from 2.9 to 2.9.2

Still not fixed in 2.9.2 . . . .

  • Version changed from 2.9.2 to 2.7

Please do not change the version tag. The Version tag allows Developers to know when the bug was first noticed.

Since this ticket is open, We know that the bug exists in 2.7~Latest Nightly.

comment:80 in reply to: ↑ 78   bsamrajni3 years ago

Replying to vinceger:

Still not fixed in 2.9.2 . . . .

Hey..I fixed it.I would be glad if u check out that and report me.

comment:81 follow-up: ↓ 82   Victor Delta3 years ago

  • Cc Victor Delta added

bsamrajni
Thanks. This bug is really annoying - I'd like to try out your fix. As a newbie, can I just confirm that I need to download your updated post.php file above and replace the existing one. Is that all one has to do? Thanks.

comment:82 in reply to: ↑ 81   cnorris233 years ago

Replying to Victor Delta:

bsamrajni
Thanks. This bug is really annoying - I'd like to try out your fix. As a newbie, can I just confirm that I need to download your updated post.php file above and replace the existing one. Is that all one has to do? Thanks.

If you're referring to bsamrajni's most recent attachment, then yes you can just replace your existing post.php with the one here. For all the other files attached to this ticket, and most others you find on Trac, this will not work.

comment:83 follow-up: ↓ 88   mtekk3 years ago

bsamrajni,

Why did you attach a full post.php rather than a .diff? Also you fix trashes permissions (sort of). I don't see this going anywhere for a while, no matter how annoying it may be (a real fix requires some rethinking of the permission system for pages, and I don't see that making 3.0).

  • Keywords early added; featured removed
  • Milestone changed from 3.0 to 3.1

Due to no traction on this, I'm moving this to 3.1 early.

A few options have been outlined, including relying on edit_private_pages & read_private_pages, but i'm not sure any are actually solid for inclusion due to the change in output from the function in question.

The same problem exists for draft pages (i.e. draft pages are not listed in the parent dropdown).

I'm wondering if (due to permissions) the draft pages version of this problem might be easier to fix?

comment:86 follow-up: ↓ 91   rooodini3 years ago

New ticket created for the draft page version of this problem: #12890

  • Cc cnorris23+wordpress@… added

comment:88 in reply to: ↑ 83   icc973 years ago

Replying to mtekk:

bsamrajni,

Why did you attach a full post.php rather than a .diff? Also you fix trashes permissions (sort of). I don't see this going anywhere for a while, no matter how annoying it may be (a real fix requires some rethinking of the permission system for pages, and I don't see that making 3.0).

Thanks @bsamrajni. I did find the whole post.php file useful as I could just apply it to my site directly.

@mtekk is right though - when you list posts / pages after putting your fix in, it displays all the draft ones as well, even when you're not logged in. On top of that it only works for draft and not private pages.

However it does provide a useful work around, until a correct fix is found. You can use the exclude pages http://wordpress.org/extend/plugins/exclude-pages/ or set the pages to be private to exclude them.

This patch seems to work around the problem in 3.0 RC 1.

  • Cc mikemcmurray added

Thank you sillybean, the patch indeed seems to work around the problem in 3.0 !

comment:91 in reply to: ↑ 86   probablepossible3 years ago

  • Version changed from 2.7 to 3.0

Replying to rooodini:

New ticket created for the draft page version of this problem: 12890

New ticket created for the pending/scheduled version of this problem; 14441

comment:92 follow-up: ↓ 93   westi3 years ago

  • Version changed from 3.0 to 2.7

Please don't change the version something was reported in to a newer version - we lose the information about how old an issue is then.

comment:93 in reply to: ↑ 92   probablepossible3 years ago

Replying to westi:

Please don't change the version something was reported in to a newer version - we lose the information about how old an issue is then.

Whoops! I apologise for the gaffe!

Refreshing patch

  • Keywords early removed
  • Milestone changed from Awaiting Triage to Future Release
  • Priority changed from high to normal

kawauso2 years ago

Refresh against 3.1-RC2-17315 and don't escape a static string

  • Keywords needs-patch added; has-patch removed

I don't think this logic belongs in get_pages(). It's too low in the stack.

Instead, get_pages() should be allowed to accept an array for post_status. Each status should probably be verified before then going into an IN() into the query.

Then the parent dropdown can call get_pages() and request private pages as well -- the cap check would need to occur at that level.

nacin2 years ago

Not tested beyond confirming it happens to work.

  • Keywords has-patch 3.2-early added; needs-patch removed
  • Owner changed from bsamrajni to nacin
  • Status changed from assigned to reviewing

This has been sitting around long enough. Accepting for potential 3.2 inclusion. Please review/test.

8592.2.diff fixes several issues:

  1. A warning in Quick Edit:
    Warning: mysql_real_escape_string() expects parameter 1 to be string, array given in .../wp-includes/wp-db.php on line 791
    
  2. Quick Edit was still showing only published pages.
  3. The change in wp_dropdown_pages() seems unnecessary.

For issue 1, I forgot to cast. Thanks for the fix. Might still prefer my solution with a string cast, to avoid an unnecessary IN when you pass array('publish'). We're pretty lame at trying to use = where it can speed things up, though I'm also not sure how current versions of MySQL handle that optimization underneath.

Same with issue 2, thanks. Issue 3: The change in wp_dropdown_pages() was indeed unnecessary, as the whole thing gets passed to get_pages(), but I think it's good to be explicit about what is accepted.

Thanks, revised the patch.

To also allow scheduled pages to be parents, and to support this in the Quick Edit as well as Edit Page areas, it appears that the post_status array must include 'future' as well in the changes to class-wp-posts-list-table.php and meta-boxes.php.

Are there other statuses that should be considered as well?

Edited: this is in response to 8592.3.diff posted by the wonderful SergeyBiryukov

Last edited 2 years ago by JohnNull (previous) (diff)

I think this fixes #4711. Will do some extensive testing next week.

I tested this patch on a site where I have a deep hierarchy of private pages and a bunch of users a) creating more pages in that hierarchy, and b) viewing lists of the private pages on the front end.

The patch solves the dropdown issue beautifully, but I'm looking at resolving #4711 and #6939 as well as this ticket. Casting $post_status as an array causes problems if you've passed more than one status in an argument string -- e.g. wp_list_pages('post_status=publish,private') returns false because the resulting array doesn't pass the array_diff() test with get_post_stati().

If, in wp-includes/post.php, instead of:

$post_status = (array) $post_status;


we do this:

if (!is_array($post_status))
	$post_status = explode(',', $post_status);

... then it becomes possible to call wp_list_pages() with more than one status, and that warning in Quick Edit is still taken care of.

Ah, yes, I forgot about supporting query string calls. I should have seen that though, because WP_Query only accepts comma-delimited, and not an array of statuses. We should be consistent. Personally, I think both should be supported in both places.

Fixing the dropdown brings up the question of what to do with the post status of a new page created under a private parent. Nacin, after you left the powwow between sessions in Phoenix, Ryan Duff and I came up with this suggestion: When the private parent is chosen, do a bit of JS to change the current page's status to private (/draft/future/etc.), gray out the visibility and/or status sections and disable their Edit links, and maybe put a tiny-print message in that section that the status cannot be changed unless a public parent has been chosen.

  • Type changed from defect (bug) to feature request

The problem there is then the private status isn't properly communicated in the dropdown.

I know this ticket has been open for years, but it's time it gets moved to a feature request, because that is what it is. There are a number of UX considerations here, which is why the functionality doesn't exist yet.

Yes, any pages that aren't public really ought to be noted as such in the dropdown, as they are in the Edit page lists.

Not sure I agree that this is a feature request; it's definitely a problem with an existing feature, even though fixing it properly is going to involve omg-bbq levels of UX work on the page Edit screen. The underlying get_pages() problem is almost taken care of. What if we finish that up, then start a new feature request ticket for the UI changes to make it clear what's going on with the newly included page statuses?

Updated the patch as per sillybean's correction.

Should we also allow scheduled pages to be parents, as suggested by JohnNull?

Adds the page's status in admin dropdown menu

Added a patch to include non-publish statuses in the admin dropdown menus.

I like the idea of including scheduled pages in this project, but I skipped them for this patch.

For me i.c.w. WordPress 3.1 http://core.trac.wordpress.org/attachment/ticket/8592/post-r16235.diff (added by sillybean) works A-Okay. Others won't ?

Last edited 2 years ago by vinceger (previous) (diff)
  • Keywords 3.2-early removed
  • Milestone changed from Future Release to 3.2

At the very least, let's get this logic into get_pages(). Closed #17056 as a duplicate.

Attached patch is a combo of Segey and sillybean. Rather than hard-coding the statuses, I took a cue from the post-row post_state logic, and added all post states to the drop down.

Refresh + combination of patches

In [17889]:

Allow get_pages() to take multiple post statuses. see #8592.

  • Milestone changed from 3.2 to Future Release

I need to punt the rest of this, but that should allow a plugin to implement the rest without bad hacks.

  • Cc wikichaves added
  • Cc inge12 added

Whew! I see that this bug has been crawling around for years!

I sure hope that "Future Release" comes up soon -- like before 3.2!

Even a plugin would be nice as a stop-gap. I see the patch, but am not sure where to insert it -- shows you how much I need the plugin or updated Wordpress release. ;)

Thanks, guys, for all your hard work. I just ran into this because I could not make private pages hierarchial and posted in the Requests and Feedback forum. (http://wordpress.org/support/topic/private-parent-pages)

  • Cc ben@… added

comment:118 follow-up: ↓ 119   sillybean2 years ago

JJJ, your updated patch to page_attributes_meta_box() isn't working for me anymore in 3.1.3 -- the parent dropdown disappears entirely. If it's working in trunk, though, I'll just upgrade to the nightly.

comment:119 in reply to: ↑ 118 ; follow-up: ↓ 122   vinceger23 months ago

Replying to sillybean:

JJJ, your updated patch to page_attributes_meta_box() isn't working for me anymore in 3.1.3 -- the parent dropdown disappears entirely. If it's working in trunk, though, I'll just upgrade to the nightly.

sillybean,

I always used your patch ( http://core.trac.wordpress.org/attachment/ticket/8592/post-r16235.diff ) and this worked A-Okay for me. Bad enough i's not working anymore with WordPress 3.2.

Is there a new or other patch like this one but will work with the new WP version ? For now I copied the POST.PHP from the previous WP-version and this works, but may I do this ?

Thanks in advance !

Last edited 23 months ago by vinceger (previous) (diff)
  • Keywords needs-patch added; has-patch removed
  • Keywords has-patch needs-refresh added; needs-patch removed

comment:122 in reply to: ↑ 119   sillybean23 months ago

Replying to vinceger:

I'm sure my old patch is too outdated to work anymore. The newer patches do work (turned out to be some weird glitch in the 3.1.3 site I was talking about), but not in the same way. You have to be explicit about which statuses you want in your loops. For example, I have this in several page templates where I need the private pages to appear:

<?php
global $wp_query;
query_posts( array_merge( array( 'post_status' => 'publish,private' ), $wp_query->query ) );
?>

Thank's for the QUICK feedback sillybean !!

Last edited 23 months ago by vinceger (previous) (diff)

Just upgraded to WordPress 3.2.1 and tried all the patches above but non of them work, some of them already implemented in 3.2.1.

The problem I have is wen I change a page, or create a new one, private pages are not available as a choice in the Parent dropdown of the Attributes module.

Please help....

Allow filtering the args for parent dropdown on Page Attributes metabox

  • Cc davecpage added

Just ran across this issue for a client. They wanted to be able to structure a large quantity of pages, with the pages still marked as Draft or Pending, but the parent page dropdowns shown within the Quick Edit or Page Attribute metabox would only show Published pages.

Fortunately there is a filter on the arguments used for the Quick Edit called 'quick_edit_dropdown_pages_args' which i've hooked into the add the other statuses (see code below). The Page Attributes metabox doesn't have any filter. So I changed the code to add one (patch provided - meta-boxes.diff ).

None of the patches previously on this ticket seem to add this filter, but it'd be nice for it to be there to match up with the Quick Edit one at least.

function show_all_pages_in_admin_parent_pages_dropdown($args) {
  $args['post_status'] = array('publish', 'draft', 'pending', 'private');
  return $args;
}
add_filter( 'quick_edit_dropdown_pages_args', 'show_all_pages_in_admin_parent_pages_dropdown' );
add_filter( 'page_attributes_dropdown_pages_args', 'show_all_pages_in_admin_parent_pages_dropdown' );

Interesting. I hadn't noticed that JJJ's patch adds only the private pages, but you're right.

To maintain support for custom statuses, we should probably use 'any' rather than enumerating each status... but when I tried that, either as a string or an array, my parent dropdown disappeared entirely and I got some weird capability errors.

Any chance JJJ's patch can go into 3.3, and we can continue working out the rest of the details after that? The piece that went into 3.2 made it much easier to display private pages on the front end, but as davecpage has discovered, it's still virtually impossible to deal with the back end issues without hacking core.

  • Keywords needs-refresh removed
  • Milestone changed from Future Release to 3.3

Refreshed for 3.3, incorporated page_attributes_dropdown_pages_args filter.

comment:129 follow-up: ↓ 136   kmklr7221 months ago

  • Cc kmklr72 added

Like davecpage, I ran into this issue for a client. I wanted to fix it without hacking core, and this function seems to be working fine (based on 3.2.1 stable):

function admin_private_parent_metabox($output)
{
	global $post;

	$args = array(
		'post_type'			=> $post->post_type,
		'exclude_tree'		=> $post->ID,
		'selected'			=> $post->post_parent,
		'name'				=> 'parent_id',
		'show_option_none'	=> __('(no parent)'),
		'sort_column'		=> 'menu_order, post_title',
		'echo'				=> 0,
		'post_status'		=> array('publish', 'private'),
	);

	$defaults = array(
		'depth'					=> 0,
		'child_of'				=> 0,
		'selected'				=> 0,
		'echo'					=> 1,
		'name'					=> 'page_id',
		'id'					=> '',
		'show_option_none'		=> '',
		'show_option_no_change'	=> '',
		'option_none_value'		=> '',
	);

	$r = wp_parse_args($args, $defaults);
	extract($r, EXTR_SKIP);

	$pages = get_pages($r);
	$name = esc_attr($name);
	// Back-compat with old system where both id and name were based on $name argument
	if (empty($id))
	{
		$id = $name;
	}

	if (!empty($pages))
	{
		$output = "<select name=\"$name\" id=\"$id\">\n";

		if ($show_option_no_change)
		{
			$output .= "\t<option value=\"-1\">$show_option_no_change</option>";
		}
		if ($show_option_none)
		{
			$output .= "\t<option value=\"" . esc_attr($option_none_value) . "\">$show_option_none</option>\n";
		}
		$output .= walk_page_dropdown_tree($pages, $depth, $r);
		$output .= "</select>\n";
	}

	return $output;
}
add_filter('wp_dropdown_pages', 'admin_private_parent_metabox');

As you might be able to tell, it is pretty much just a copy of wp_dropdown_pages from /wp-includes/post_template.php with a few customizations. It could probably do with being cleaned up, but it seems to be doing what I want for now. Hopefully it will help someone here.

+1 on 8592.2.patch for 3.3.

  • Keywords commit added
  • Resolution set to fixed
  • Status changed from reviewing to closed

In [18818]:

Add filter for the args into wp_dropdown_pages() in the page attributes box. Give the list_pages filter the context of the post object. fixes #8592 for 3.3.

  • Milestone changed from 3.3 to Future Release
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Type changed from feature request to enhancement
  • Cc mattheweppelsheimer added
  • Cc mike@… added

comment:136 in reply to: ↑ 129   vinceger18 months ago

  • Version changed from 2.7 to 3.3

'Thank's 129 kmklr72 !

This did the trick with 'my problem' , other patches won't work here...

Replying to kmklr72:

Like davecpage, I ran into this issue for a client. I wanted to fix it without hacking core, and this function seems to be working fine (based on 3.2.1 stable):

function admin_private_parent_metabox($output)
{
	global $post;

	$args = array(
		'post_type'			=> $post->post_type,
		'exclude_tree'		=> $post->ID,
		'selected'			=> $post->post_parent,
		'name'				=> 'parent_id',
		'show_option_none'	=> __('(no parent)'),
		'sort_column'		=> 'menu_order, post_title',
		'echo'				=> 0,
		'post_status'		=> array('publish', 'private'),
	);

	$defaults = array(
		'depth'					=> 0,
		'child_of'				=> 0,
		'selected'				=> 0,
		'echo'					=> 1,
		'name'					=> 'page_id',
		'id'					=> '',
		'show_option_none'		=> '',
		'show_option_no_change'	=> '',
		'option_none_value'		=> '',
	);

	$r = wp_parse_args($args, $defaults);
	extract($r, EXTR_SKIP);

	$pages = get_pages($r);
	$name = esc_attr($name);
	// Back-compat with old system where both id and name were based on $name argument
	if (empty($id))
	{
		$id = $name;
	}

	if (!empty($pages))
	{
		$output = "<select name=\"$name\" id=\"$id\">\n";

		if ($show_option_no_change)
		{
			$output .= "\t<option value=\"-1\">$show_option_no_change</option>";
		}
		if ($show_option_none)
		{
			$output .= "\t<option value=\"" . esc_attr($option_none_value) . "\">$show_option_none</option>\n";
		}
		$output .= walk_page_dropdown_tree($pages, $depth, $r);
		$output .= "</select>\n";
	}

	return $output;
}
add_filter('wp_dropdown_pages', 'admin_private_parent_metabox');

As you might be able to tell, it is pretty much just a copy of wp_dropdown_pages from /wp-includes/post_template.php with a few customizations. It could probably do with being cleaned up, but it seems to be doing what I want for now. Hopefully it will help someone here.

Version 2, edited 18 months ago by vinceger (previous) (next) (diff)
  • Keywords needs-refresh added
  • Version changed from 3.3 to 2.7

Version number indicates when the bug was initially introduced/reported.

  • Cc chappellind@… added

Definitely needs refresh. I discovered another issue today where if you assign a page as a child of another page, and then set that parent page's status to private, the child page is still associated with the parent but the dropdown list on the child pages shows (no parent).

  • Cc seancojr added

Just updated to WordPress 3.3.2. and still without success. This ticket opened 3 years ago :-(

  • Keywords needs-refresh removed

I have tried this patch - which is sorely needed - and it is working fine for me. Thanks!

  • Keywords 3.5-early added
  • Cc knut@… added

This problem is also present with the Options - Reading - Posts Page dropdown. Do I need to create a separate ticket for that?

Update: Yes, I will have to. I have tested the latest patch and it fixes this issue, but not the issue on Options Reading.

Last edited 11 months ago by knutsp (previous) (diff)

The latest patch uses the capability 'read_private_posts' to check if status 'private' should be included in the list. Should it not check the 'read_private_pages' capability instead?

Closed #21205 as a duplicate.

  • Cc manuel@… added

Roundup of where we stand on this issue as of 3.5b3.

Things that can be fixed with filters:

  1. Add private/draft/future/pending pages to parent dropdown in page attributes metabox and Quick Edit
  2. Add (status) to titles in page parent dropdowns
  3. Filter front-end page loops to include privately published ones.
  4. Add all statuses to the menu admin screen's page metabox.
  5. Filter lists of pages (wp_list_pages, wp_page_menu, and menu functions) to include privately published ones.

See https://gist.github.com/4091448 for examples.

Items 1 and 2 are covered by 8592.3.patch (which probably should be changed to use the 'read_private_pages' capability as knutsp pointed out).

Item 3 uses a well-known mechanism for altering loops, and I think we can leave that alone.

I do think we need to do something to fix item 4. My example doesn't include the "(status)" labels, but a patch probably should.

My example solution to item 5 is inefficient, since we have to wait until the output is generated before we can overwrite it with new arguments. I think it would be better to either add a filter on the arguments array in wp_list_pages(), or put in a few lines to let us filter the post_status alongside the excluded page IDs.

Things that can't (easily) be fixed with filters:

  1. The page dropdown in Settings -> Reading. You'd have to go through the same contortions as in item 5, with the addition of an is_admin() conditional. (Plus, in my testing, it doesn't work, although that could be a quirk of my test setup or a bad cache or who knows.)

I'm ambivalent about "fixing" 6, as it's the most likely to lead unwary users into doing something they didn't intend. On the other hand, adding an argument array filter in wp_dropdown_pages() would let developers do it if they really, really wanted to, and might provide flexibility in other areas beyond the scope of this ticket.

Have I missed any other places where other page statuses need to be included?

What needs to be done to get this issue fixed in 3.6? I think:

  • Refresh the patch and change the capability
  • Write a patch to cover the page metabox in the nav menu admin screen, preferably with (status) titles, keeping in mind that plugins can register custom statuses.
  • Discuss whether to filter the entire argument array in wp_list_pages() or just the post_status, and write a patch accordingly.
  • Discuss whether to do the same for wp_dropdown_pages().
  • Cc miss_jwo added

Could someone explain how to implement the patches/fix that sillybean mentions?

  • Cc robwil0 added
Note: See TracTickets for help on using tickets.