Page::trashable returns false for the current page for the Superuser

[schwarzdesign]

Short description of the issue

On a default install (current dev branch, blank profile), $page->trashable returns false for the superuser, unless the core permission page-edit-trash-created is manually installed.

Expected behavior

The superuser should always be able to delete any page, so $page->trashable should always return true for the superuser.

Actual behavior

The permissions check fails unless the page-edit-trash-created permission is installed on the site.

Optional: Screenshots/Links that demonstrate the issue

Forum post and detailed explanation here.

Optional: Suggestion for a possible fix

Ok so I have dug deep and determined why it isn't working. The $page->trashable() method is added by PagePermissions.module as a hook. The method PagePermissions::trashable first calls $this->deleteable(), which returns false for the current page (so far expected and documented behaviour). However, the $this->wire('permissions')->has('page-edit-trash-created') check in this line fails, since the page-edit-trash-created permission doesn't exist in a default install. Since it directly calls the permissions fuel whose has method doesn't check for superuser, and the check fails if the permission doesn't exist. If I either create the permission through the backend or replace the above check with $this->user->hasPermission('page-edit-trash-created'), it works as expected.

This is my suggested fix (for PagePermissions.module#L734):

// current
if(!$event->return && $this->wire('permissions')->has('page-edit-trash-created') && $page->editable()) {

// change to
if(!$event->return && $this->wire('user')->hasPermission('page-edit-trash-created') && $page->editable()) {

Steps to reproduce the issue

1. Install ProcessWire (dev branch, blank profile)
2. Check $page->trashable for the current page as the superuser
3. The method returns false

Setup/Environment

• ProcessWire version: 3.0.130
• PHP version: 7.2
• MySQL version: 5.7

[Toutouwai]

I can confirm the issue, but I don't think the cause is line 734. This line tests to see if the page-edit-trash-created permission exists (i.e. if it is installed). If it isn't installed then it's not possible for any role to have that permission and so there's no need to proceed any further with that section of code.

I think the issue was introduced in this recent commit.

Should there be a distinction between whether the currently viewed page may be deleted and whether it may be trashed?

And it also raises the question of what $page->trashable() is supposed to be used for. Is it meant to determine "in general, does this user have the necessary permissions to trash this page", or is it meant to determine "may this user trash this page right now in the current context".

[schwarzdesign]

@Toutouwai Ok so maybe the function should return true earlier? I'd say the superuser should always pass any permission checks.

And it also raises the question of what $page->trashable() is supposed to be used for. Is it meant to determine "in general, does this user have the necessary permissions to trash this page", or is it meant to determine "may this user trash this page right now in the current context".

For what it's worth, in my context I just want to know if the user has the general permission to trash a page in order to display a link to the edit page with the trash tab opened. So in that sense, for my usecase it's independent of the current context. It's also counterintuitive to have a permissions check depend on the current context, so I'd say it should return true if the user can use the trash tab, regardless of the context.