Permissions

Try the attached patch and let me know your thoughts. To use it, first go to admin/structure/empty-page/settings and enable the access control setting. After that, you should be able to set role-based permissions on each of your callbacks.

I originally wrote this patch so that I could restrict access to custom admin pages I had created composed entirely of blocks. But it turned out not to be so useful since the block module's access system doesn't respect the node-access system, and the blocks still showed up on the 403 page until I manually went through each block and configured their visibility settings.

Someone could possibly pick this up and write some handlers to propagate the access-control settings to blocks whose URL is the same as the given callback, but in my case I'm gonna try giving the context module a shot instead.

Hope it helps!

Yeah I considered this at the beginning. But I decided that I didn't want to try to duplicate other modules. If you want features like that, I'd recommend trying ctools Page Manager. That being said, if it makes sense to allow basic customization of hook_menu $item (title, description, menu name, context, etc.) and various access control options, then maybe it should be in the add-on module.

Per your comment Nick, I've reworked the previous patch to move the access-control routines to the empty_page_extras module.

My aim was to make as little impact on the main module as possible, though I altered the load/save routines in order to handle $callback->data as a serialized array.

Let me know what you think.

Updated the patch with a separate uninstall routine for the empty_patch_extras module.

Please review this one instead of #4; I forgot to do "git diff HEAD" so it didn't include the newly-added empty_page_extras.admin.inc & empty_page_extras.install files.

By in large I think this looks really good. A couple of minor questions/points below.

+++ b/empty_page.moduleundefined
@@ -136,12 +142,24 @@ function empty_page_get_callbacks() {
+    $callback = (object) array_map(create_function('', ''), array_flip($fields));

Why are we returning an empty(ish) callback object? Or asked a different way, why would we call empty_page_get_callback with a null $cid

+++ b/modules/empty_page_extras/empty_page_extras.admin.incundefined
@@ -0,0 +1,35 @@
+    '#type' => 'fieldset', ¶

whitespace

+++ b/modules/empty_page_extras/empty_page_extras.admin.incundefined
@@ -0,0 +1,35 @@
+    '#type' => 'checkbox', ¶

whitespace

+++ b/modules/empty_page_extras/empty_page_extras.infoundefined
@@ -2,3 +2,5 @@ name = "Empty Page Extras"
+dependencies[] = empty_page

nice catch ... oops :)

+++ b/modules/empty_page_extras/empty_page_extras.installundefined
@@ -0,0 +1,45 @@
+}

"no new line at end of file"

Thanks for the review. RE: "why would we call empty_page_get_callback with a null $cid"

1. It follows the Null Object pattern, which makes for good reusable design and, well... I've been reading Martin Fowler.
2. This function gets called with a NULL value in empty_page_extras_callbacks_form_submit() (near the end of the patch file). Doing so reduces the amount of code needed to switch between processing a new callback/updating an existing callback down to one ternary operator. I've heard you're partial to 'em.

Here's the updated patch.

Confirmed this patch works as advertised.

Yes, work great!

Nice work. Thanks for the help. Seems to work well.

Question: In function empty_page_uninstall() why replace drupal_load('module', 'empty_page'); with module_load_include('module', 'empty_page');? I don't get any errors when uninstalling empty page that would imply the original is not working. Am I missing something? (Fyi, this part of the patch failed for me because your patch did not remove the original drupal_load('module', 'empty_page'); line. Maybe you were missing this and is why you got the error?)

Also, I don't really see a need for the "Enable role-based access control" checkbox in settings. The user has already enabled the extras module. I don't see a need for yet another opt-in.

Thoughts?

More food for thought...

At least, shouldn't Empty Page emit a 403 (i.e. call drupal_access_denied()) when the current user's role doesn't have the "access content" permission?

The module produces "pages" (albeit empty ones), which ordinarily are not accessible by users without "access content" in Drupal. So, does this make sense?

My use case: I'm developing a private site where as content can be accessed only by logged in users. In other words, only the "authenticated user" role has the "access content" permission.

I tried to use the module to provide some empty pages with only (restricted) blocks, to no avail. The page is accessible to anonymous users, instead of showing my 403-"you must login" page.

(Also, Drupal won't return a 403 HTTP header on such use cases. This allow search engines to index the -- empty -- page, which can create duplicate-content problems on SEs.)

This patch save my life. Nice work.