Sorry, you are not allowed to access this page when activating Social Meta module

[wpsmort]

When activating the Social Meta module, the first time you click on the Social Meta menu item you get a Sorry, you are not allowed to access this page error.

This is because the menu link is http://testsite.dev/wp-admin/admin.php?page=all-in-one-seo-pack/modules/aioseop_opengraph.php when it should be http://testsite.dev/wp-admin/admin.php?page=aiosp_opengraph

Need to find when this broke.

[arnaudbroes]

This started with 2.3.15
So apart from this issue, the weird thing is that the Social Meta module file "aioseo_opengraph.php" is located in "/all-in-one-seo-pack/modules/aioseo_opengraph.php".

When a page is added to the submenu, the menu item refers to "admin.php?page=all-in-one-seo-pack/modules/aioseop_opengraph.php" while the working slug is "admin.php?page=aiosp_opengraph".

A few other menu items refer to "/modules/%module_name%.php" while some others refer to "%module_name%.php", even though all of them are located in /all-in-one-seo-pack/modules/...
So apart from this issue, I think we should take a look later on how this is happening.

[michaeltorbert]

In aioseop_module_class.php function add_menu() there are three add_submenu_page()s.

The first one, on line 1971 is how most modules get added.
The 2nd one (default) is for General Settings.
The third one is how Social Meta and Robots.txt get added. The difference the 6th parameter, $this->get_prefix( $k ) . $k vs plugin_basename( $this->file )
Subbing in plugin_basename( $this->file ) works. We need to figure out if this was done on purpose or not.

[wpsmort]

@michaeltorbert I tracked this bug back to this commit - 98703b5

[michaeltorbert]

I'm not sure why that PR tagged this issue, but it's unrelated and can be ignored within the context of this issue.

[michaeltorbert]

The problem is the settings being moved to a new init() function on the WP init action hook. This has the information needed to add the submenu, which needs to be available before the init function runs.
@amostajo Do you have any idea why this was moved?

[amostajo]

@michaeltorbert This was moved under "init" hook because the module was being initialized before wordpress or external plugins were initialized/instantiated, this was causing an scope issue, because the module was limited to default post types and taxonomies. Custom types/taxonomies registered by other plugins were left out of the scope.

Can't recall if this was related when doing the taxonomies task, but custom things registered by 3rd party plugins where not showing in the checkboxes because the menu/submenu was being genereated before wordpress initializes itself and all plugins.

[michaeltorbert]

@amostajo What are your thoughts on how we should handle this?

1. Swapping $this->get_prefix( $k ) . $k with $k vs plugin_basename( $this->file ) as I described above fixes it, but I'm not sure what else that may break.

2. Completely rewrite how the menus are created. (Long overdue, but that may be overkill just to fix this).

3. Move the code back to the constructor from init(). (What would this break? Is there another way to fix those other than by moving this code? If this stuff is running to early, keep in mind that the info is stored in a global variable.)

4. Simply add $this->init() to the constructor. (Not sure if this breaks anything, but it seems kind of an ugly way of doing it. Surely there's a better way.)

[amostajo]

@michaeltorbert The right thing to do would be option (2), imo. Because menus should be created following Wordpress initialization flow. Menus should not be created before Wordpress initializes itself and plugins.

My comments to the other options:

Swapping $this->get_prefix( $k ) . $k with $k vs plugin_basename( $this->file ) as I described above fixes it, but I'm not sure what else that may break.

No sure... without unit testing this is unknown... and every little change in this plugin always affect something, my past experience...

Move the code back to the constructor from init(). (What would this break? Is there another way to fix those other than by moving this code? If this stuff is running to early, keep in mind that the info is stored in a global variable.)

The issue reported were custom post types/taxonomies are not showing will reappear. Cuz it is initializing menus before wordpress init.

Simply add $this->init() to the constructor. (Not sure if this breaks anything, but it seems kind of an ugly way of doing it. Surely there's a better way.)

Same issue as below. The only difference is that part of the code is on a method rather than constructor.

[michaeltorbert]

I agree 2 is the best solution, at least long term, but it may take quite a bit to do it. I've never liked how complicated the menu code is. It's normally pretty simple in WP plugins. We should probably first figure out why the behavior of 1 exists in the first place though before we ditch it completely.

For 4, I meant that we call init() on the init action, as well as from the constructor (as a temporary solution). I'm not sure if that causes any problems.

[amostajo]

Will test it

[amostajo]

Did some testing and appears to work fine without any errors thrown.
@wpsmort This might need deep testing though not sure what may affect, would you like a PR for pro as well?

[michaeltorbert]

@amostajo Is there a PR for this?
Since this is pretty simple code-wise, just a PR in free will probably suffice, and for Pro testing @wpsmort can just copy/paste the line of code. Testing needs to be done in free and Pro... it's unclear what, if anything, this may affect.

[wpsmort]

PR #1082 has been tested and is good.