Posted by Gábor Hojtsy on February 6, 2008 at 2:32pm
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | menu system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
On Drupal.hu, after the Drupal 6 upgrade, we noticed that unpublished book pages show up in the book outline. Look at http://drupal.hu/kezikonyv/bevezetes and click the last link, which is an unpublished node. I tried to save the node to check if it clears off its database data, but it still shows up there.
Here is an attached shot of the book outline editor showing the unpublished node as well.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| unpublished_in_book.jpg | 57.79 KB | Ignored: Check issue status. | None | None |
Comments
#1
I can reproduce this with a clean copy of D6 and two book pages, one a child of the other and unpublished.
#2
Hm, looks like http://api.drupal.org/api/function/_book_update_outline/6 is not aware of whether the node is published or not. Also, I wonder how are node level permissions handled in book module if at all, hm.
#3
@Gabor - node level permissions are checked via db_rewrite_sql (node_access). So, it cold be a bug there? It's the same checking as done for the menus.
Try putting a link to this node in of the site menus.
The other possibility is that we need to add a check on node.status to the SQL here:
http://api.drupal.org/api/function/menu_tree_check_access/6
I had assumed that the rewrite code took care of unpublished nodes, but maybe that is not the case:
http://api.drupal.org/api/function/_node_access_where_sql/6
#4
#5
I added my unpublished child page from my test #1 to the Primary links menu, and logged out. The link (to the unpublished node) is visible to anonymous users (but returns "Access denied" when clicked).
#6
So it looks like menus are not checking for the published state of nodes at all.
#7
#8
If they are using db_rewrite_sql(), they will not be checked for users with 'administer nodes' permissions. So you would have to put an explicit status check into the query.
On the other hand, there are valid reasons why users with 'administer nodes' should be able to view unpublished nodes.
We should check this behavior against D5. As it might be "by design" if unexpected.
#9
UNTESTED patch, but obvious and simple code change.
#10
see also:
http://api.drupal.org/api/function/node_page_default/6
other code adds a check for status = 1 which is not bypassed for admins.
and thus it was in D5 for book module:
http://api.drupal.org/api/function/book_tree/5
#11
The menu thing (a visible menu item pointing to an unpublished node linking to an "Access denied" for anonymous users) also happens in D5.
I tested the patch in #9 on my simple test case, and it works fine, and solves both the "menu issue" and "book issue".
The only way that I could get a link to the unpublished node to appear with the patch applied is to create a three-tier hierarchy and then unpublish the node in the middle. The "up" link on the bottom node then points to its parent, which is unpublished. And, since even admin's don't see unpublished nodes in the interface (outline, hierarchy, etc.) you obviously can't manage the unpublished nodes as part of the book.
I'm not really sure what the expected behavior is in this situation (administering unpublished book pages as an administrative user). The patch solves the immediate problem (and also fixes the menus, which is nice) rather handily though.
#12
@keith.smith in D5 there was no access check when menu links were displayed, that's a major new feature of the D6 menu system.
#13
Can also confirm #9 patch keeps unpublished book pages from showing in the book outline structure and when set as menu items.
#14
#9 -- Hides the unpublished nodes, but...
Children of unpublished nodes disappear from the book outline (are no longer displayed on admin/content/book/# or on other pages), and lose all book navigation except the 'up' link to the unpublished parent.
I'm not sure whether that should be 'by design', or if it should be fixed.
Also -- Maybe check if the user has access to the node and display the link (with class = "unpublished")?
#15
Dont show link even with class=unpublished. And disappearing children, ah well, that's "no we are not fixing" its too deeply ingrained. I am not debating whether its good design or not, lets get back in D7. The patch is fine.
#16
Unpublishing stuff in the middle of the book structure and then the 'up' link pointing to the unpublished node sounds like a minor issue compared to exposing unpublished stuff in menus and books. So I committed this for now to plug in this hole. I would not be surprised though if we would revisit the "unpublished in the middle" issue later in D6.
#17
Doh, #9 still needs to be committed to 7.x
#18
Committed to CVS HEAD. Thanks.
#19
Automatically closed -- issue fixed for two weeks with no activity.
#20
Hi,
I'd like to show unpulished items in the book hierachy table ( page admin/content/book/ ), How can i do it ?