Security page feedback and improvements

I hook it to admin_menu. But don’t worry about me; I’m happy to attach my function to the settings menu instead anyway. (I have it coded so it will work on WP this way; I can just remove the conditional that checks for the CP menu item.)

1 Like

So it seems to have been the common approach for some reason to use admin_menu

I personally would just ditch the page, but there have been concerns about the semver breaking if we do just remove something that existed as an API… and even if no one would care, this is actually true.

So I wanted to find a solution that allows us to safely remove this, without actually breaking anything and keeping even the functionality for those who want it.

Seems my plan is not going to work out, unless we would accept that since the DOC never specified the hook to add it… we could update the doc and say the plugins need to update the code.
It would not break the current installs, so they could prepare for it before CP even ships the change!

admin_menu is the correct hook because it’s on the admin menu! But I’d be OK with ditching the whole page; I don’t think anyone else uses it. (We did actually ask that some time ago, and no-one else responded.)

1 Like

If that is actually all that is needed, I have would have a solution for that.
I would however mean we expect the function to be loaded on admin_menu and otherwise it is doing it wrong.

If we can agree on that we should update the DOC and … the change I made will work, I just need to unset based on the submenu presence, instead of the menu’s presence.

It makes sense the admin_menu is the right hook because if we use init the sequence of menus are actually inverted (putting the plugin menu above the CP core one in the submenus), and the $submenu global becomes the plugin menu as keynote, which is also wrong

I pushed the change to GIT, it should now eliminate the menu correctly, as long the plugin uses admin_menu hook for this function, which would be the right thing to do if I get it right.
We also should update the doc, in any case, right?

2 Likes

That all sounds good to me.

1 Like

The DOC is updated, and @MattyRob already put some Unit Tests in place, so I guesshope we can bring this in next release…

This should probably have been a Petition, but then, nothing breaks or changes…

Case closed.

3 Likes