Replace is_plugin_page()/double loading with action hook

A more detailed explanation of the issue:

Currently a given plugin is loaded twice when viewing the plugin's Options page: once at initialization along with all the other plugins, and again to output the Options page. The is_plugin_page() function was introduced to test which load is being processed, so plugins know when to output their Options page and when to just load normally.

The whole double-loading issue is confusing to many plugin developers, who often times don't even know is_plugin_page() exists, and resort to putting if (!function_exists()) around every one of their functions just to prevent function redeclaration errors.

I'd like to propose that the is_plugin_page() function and the Options page double-loading be deprecated in favor of an action hook (say, 'options_page_<pluginname>'), so plugin developers can simply write an action function to output their Options page. This would be much easier to explain to plugin developers, and there would be no need to re-execute the entire plugin (with preventative testing) just to output the Options page.

Per discussion with Matt, the new action hook will be implemented.

The hook tag will have the format 'options_page_<pluginname>', where <pluginname> is the plugin's filename with the '.php' stripped. For example:

add_action('options_page_multilingual', 'ml_options_page');

...for my multilingal.php plugin, or:

add_action('options_page_mini-posts', 'mp_options_page');

...for my mini-posts.php plugin.

Backward compatibility will be maintained with the double-loading method of doing things.

edited on: 02-03-05 04:43

In a message to the hackers@ mailing list, Owen brought up the fact that the patch does not address custom plugin Management pages.

I've submitted a new patch which accommodates this, and additionally resolves bug #797, one that I'd been meaning to submit for a while. ("Plugins can't have separate Options and Management pages.")

It should be noted that this required moving plugin page handling from admin.php to both options.php and edit.php, meaning any plugins which currently submit or redirect to admin.php?page=pluginname.php (including my own) need to be updated to use options.php?page=pluginname.php and/or edit.php?page=pluginname.php, depending on whether they use Options or Management pages, respectively.

The patch does attempt to maintain backward compatibility with plugins which expect to use admin.php -- however, it can only assume that plugins which have updated to the new action hook method have also updated to use options.php and/or edit.php. Plugins which have done the former but not the latter will still break, but there's simply no way of knowing which page the plugin expects to submit to. And anyhow, Owen's the only one whose plugin would break right now, since he already updated Geo to use the action hooks. (Sorry Owen.)

(Note: I've updated the patch a couple of times while writing this, so if by some wild chance you happen to have downloaded it in that time, download it again.)

(I should probably watch this bug, huh?)

I figured out a way to get Options and Management pages in the same plugin, but it uses code from menu_header.php. What should really happen - rather than making changes to several pages - is the menu code should parse the page query value out of the query string (ie. 'something.php' from '.../admin.php?page=something.php') and use that to determine what file to include rather than using the value stored in the array created by add_options_page/add_management_page. That way, you could pass additional parameters in the query string ('.../admin.php?display=whatever&page=something.php') when creating the menu, and you could use those parameters to display the correct page output using the new hooks. This would be useful for other passing other values, too, which are difficult now.

I'm not doing a good job of describing this, but my main point is that there should be a way to affect this change without changing every admin file, and I think the solution it located in the menu code itself.

Philosophically, it's currently sufficiently difficult to add admin pages in more than one place with a single point of plugin activation, but how many developers are going to do this that it needs to be made easy? Well, I'd appreciate it, but so what, right? ;)

As far as Geo goes, it's the trunk version and not the tagged version that uses the changes, so it's no big deal.

<ringmaster>: What should really happen - rather than making changes to several pages - is the menu code should parse the page query value out of the query string (ie. 'something.php' from '.../admin.php?page=something.php') and use that to determine what file to include rather than using the value stored in the array created by add_options_page/add_management_page.

It does look at the query string to determine which file to include -- look at show_plugin_page in the patched admin-functions.php. And anyhow -- what's wrong with making changes to several files? I would think that making something work logically, rather than simply trying to minimize the number of affected files, would be the only thing of importance. The number of affected files seems irrelevant to me.

<ringmaster>: That way, you could pass additional parameters in the query string ('.../admin.php?display=whatever&page=something.php') when creating the menu, and you could use those parameters to display the correct page output using the new hooks.

I did consider simply adding another value to the query string, but I didn't want to add complexity to plugins' query string munging. (Even having one to deal with adds complexity, but there's no reasonable way around that.) That coupled with the fact that I think using admin.php for displaying the custom page is a bit kludgy, since the filename is insufficiently descriptive of what the page being viewed is: it's either going to be an Options page -- which should have "options" somewhere in the filename -- or it's going to be a Management page -- which ideally should also have "manage" somewhere in the filename, but for legacy reasons all management pages have 'edit' in the title, which is a sufficient compromise. And I wasn't about to suggest that all management filenames be changed. :-)

<ringmaster>: This would be useful for other passing other values, too, which are difficult now.

I don't follow this. You seem to be saying that it would be easier to pass values into the query string if we added a 'display' value as well, which I don't follow; but then you say that passing values in the query string is currently difficult? I don't understand. :-/

<ringmaster>: Philosophically, it's currently sufficiently difficult to add admin pages in more than one place with a single point of plugin activation, but how many developers are going to do this that it needs to be made easy? Well, I'd appreciate it, but so what, right? ;)

Are you saying that because most plugins won't take advantage of the functionality that we shouldn't make it easy to implement? I see that as completely backwards logic. Usability applies to developers too; if something's not easy to implement, the number of developers taking advantage of that feature will drop drastically. Granted, it's a more advanced feature that most plugins won't *need* to use regardless of how accessible it is. But to say that the fact that a feature is more specialized justifies not making it easy to use is the wrong way to go about designing an API, I think.

<ringmaster>: As far as Geo goes, it's the trunk version and not the tagged version that uses the changes, so it's no big deal.

Oh okay, I didn't check that. :-)

edited on: 02-04-05 20:59

I'll try this again since I was bunglingly incomprehensible...

Look in menu-header.php around line 15:

if ( file_exists(ABSPATH . "wp-content/plugins/{$item[2]}") )

When you call add_options_page(...), the fourth parameter (usually the filename of the plugin) is used for the value of {$item[2]} in the line above.

If instead of passing the filename of the plugin, you could pass, literally:

'admin.php?page=pluginfile.php'

This would load the plugin even with the current code. The only problem with that is that later (line 38 or so) when it tries to figure out what menu set to use, it looks (incomprehensibly) at the first 10 characters of $item[2] to determine the submenu set to use. What it *should* do is parse $item[2] for the value of 'page', and use *that* (or the entire string if there are no parameters, which would mimick the default behavior).

If that's what it did, then you could pass as the fourth parameter of add_options_page:

'admin.php?page=pluginfile.php&display=ops'

When the plugin executed its options_page_pluginfile hook, it could test the value of $_GETdisplay? and produce the proper output page for the menu that was chosen. Note that 'display' is unique to the plugin, and the name of this querystring parameter is irrelevant to WordPress code.

You would simply call add_options_page/add_management_page with different values for 'display' to add multiple menus.

Fundamentally what I'm saying is that it's the menu system's fault that this is difficult to do, so fix the menu routines instead of making changes across several files. The problem with making changs across several files is that you'll be introducing more points of failure. Without proper code remarks, changes to one file may not propagate to changes in the others. Also, you this code wouldn't work from other admin pages if they are allowed custom admin menus, which changing the core menu operation would.

Using 'options.php?page=' or 'manage.php?page=' seems like it makes sense, but the end result is transparent to the plugin developer anyway. If they call the add_X_page function with the correct output file, the correct menu will appear, no matter what the URL is. The developer need not know/care what page produces the final output, so why go through the trouble of aiming it at a specific file?

Looking towards a solution from what the developer should have to do, a better bet for the whole shebang would be to change add_options_page to include a parameter that specifies the value to use when calling the options_page_* action hook. Currently it's unused, right? Imagine this:

add_options_page('My Plugin', 'MyPlug?', 5, 'myp.php', 'ops'); add_management_page('My Plugin', 'MyPlug?', 5, 'myp.php', 'man'); function options_page_myp($section) { if('ops'==$section) echo 'Options interface'; if('man'==$section) echo 'Management interface'; }

Now *that* is simple.

Actually, something else that bugs me about this thing is that I can't use FILE anymore for plugin references. I often copy a plugin file from test.php to test2.php just so I can try some things out and scrap them if they don't work.

With this new hook, I have to use function names that are dependent on the filename. If the filename changes, they don't work any more without making a few small changes. Few small > none.

Also, have you noticed that you can put your plugins in a directory inside plugins (this is *great* for multi-file plugins) and WP still finds them? But you can't have a function name with a slash in it, so can you even use the new options_page_* hook in this case?

I personally don't care what they say about putting everything in one file - the plugin I'm writing is frikkin' huge, and there's no way I can logically organize it into one file. A directory that houses the whole thing works very well for this.

Alright, I see what you're saying now about changing the $file parameter to add_*_page(). I still don't think it's the best option, though, since coding:

add_options_page('My Plugin', 'MyPlug?', 5, 'admin.php?page=myp.php&display=options');

...is redundant. Devs shouldn't need to specify a display of options when there's already a distinction between add_options_page() and add_management_page(). It's extraneous. Likewise with:

add_options_page('My Plugin', 'MyPlug?', 5, 'myp.php', 'ops');

Of course, that could be fixed in the add_options_page() and add_management_page() functions themselves, before they call add_submenu_page() -- but then you'd get into more query string munging, which I still think is kludgy.

I think it's easy to say that "we'll just add some parameters, it'll be fine." But the API can get real cluttered real fast with those kinds of quick fixes.

<ringmaster>: With this new hook, I have to use function names that are dependent on the filename. If the filename changes, they don't work any more without making a few small changes. Few small > none.

To be more correct: it's not the function names that are dependent on the filename, it's the hook tag. Seems like a pretty minor drawback to me, compared to the kludginess of the double-loading system, and one easily remedied by composing the action tag name on-the-fly:

$action_tag_name = str_replace('.php', , plugin_basename(FILE)); ... add_action('options_page_' . $action_tag_name, 'my_options_function') add_action('management_page_' . $action_tag_name, 'my_management_function')

<ringmaster>: Also, have you noticed that you can put your plugins in a directory inside plugins (this is *great* for multi-file plugins) and WP still finds them? But you can't have a function name with a slash in it, so can you even use the new options_page_* hook in this case?

Again, it's not the function name that would have the slash in it, it's the action tag. The action tag can have anything you want to in it. And anyhow, the file specified isn't loaded directly, so there's no need to use any sort of directory structure in the action tag. You can even do this:

add_options_page('My Plugin', 'MyPlug?', 5, 'myplugin'); // note the lack of .php file extension add_action('options_page_myplugin', 'my_action_function');

No filename has been specified at all, since no file is loaded. It doesn't matter which file my_action_function() is in, the only thing that matters is that add_action() and add_options_page() are called with same tag, or portion thereof, respectively.

Likewise, the page URLs can be called with '?page=myplugin' and it would still work. The filename is not relevant.

<ringmaster>: I personally don't care what they say about putting everything in one file - the plugin I'm writing is frikkin' huge, and there's no way I can logically organize it into one file. A directory that houses the whole thing works very well for this.

Well, personally I intend to keep my plugin contained in one file, but I guess that's just personal preference. I don't know how large yours is, but mine is currently at about 2500 lines and I expect it to be around 5000 when it's completed. Tough to keep straight in an editor, that's for sure, but much easier on the end user, and I'm all about the usability, baby. ;-)

<morganiq>: Devs shouldn't need to specify a display of options when there's already a distinction between add_options_page() and add_management_page(). It's extraneous.

Sure, but then you can't have more than one management menu in a single plugin, either. And once again we're getting to the point where I ask "who the heck are we kidding trying to make this easier for two users to code?" ;)

<morganiq>: To be more correct: it's not the function names that are dependent on the filename, it's the hook tag. Seems like a pretty minor drawback to me, compared to the kludginess of the double-loading system, and one easily remedied by composing the action tag name on-the-fly

Ah, yes. That's my own stupid fault for using the hook names as function names inside a class. Makes it easy to find the functions used for the specific hooks. You could surely do as you suggest.

<morganiq>: Well, personally I intend to keep my plugin contained in one file, but I guess that's just personal preference. I don't know how large yours is, but mine is currently at about 2500 lines and I expect it to be around 5000 when it's completed. Tough to keep straight in an editor, that's for sure, but much easier on the end user, and I'm all about the usability, baby.

I just started rewriting this one, and just the non-functional framework is around 700 lines. I need multiple files. Besides, is uploading a single directory of files so much less usable? Bah... I'm doomed.

Just tell me what I have to do to get a single point of activation (on the Plugins page) produce admin console menu insertions in both Management and Options, and I'll be fine.

I uploaded a new patch that basically does what Morgan's does but does not require modifying edit.php or options-general.php. Instead of targeting Management and Options, it applies generically to all menus. Here's the gist:

In menu.php, create an array of parent files and their page hooks. Their page hooks are created by running their menu titles through sanitize title.

In admin.php, look to see if a page hook is set for the given plugin and parent. If so, call the hook and exit.

In menu-header.php, instead of linking to the plugin page through admin.php, link through the parent file if a page hook is set. So, the old way of admin.php?page=plugin-page.php becomes edit.php?page=plugin-page.php. Since all admin pages load admin.php, all pages are capable of being used as a plugin page loader. admin.php is still used for those pages that do not specify hooks.

add_options_page() and add_management_page() return the plugin hook name for the page added. This hook can then be fed to add_action().

A sample plugin, menu-test.php, is attached.

edited on: 02-06-05 06:08

edited on: 02-06-05 06:09

We could also have add_options_page() accept an optional arg that specifies a callback function to register. If this is passed, add_options_page() will do the add_action for you.

function options_page() {

echo "<h2>Test Options</h2>";

}

add_options_page('Test Options', 'Test Options', 8, FILE, 'options_page');

I like that. I'll update my patch.

edited on: 02-06-05 06:37