admin.php should properly set "now" globals

[wonderboymusic]

There are a handful of variables in admin.php which are sometimes declared, sometimes not. Whether they are declared are not, they are imported into other functions/methods as globals.

Begs these questions:

Which globals are set in admin.php?

$hook_suffix, $plugin_page, $typenow, and $taxnow

How many of them have the global keyword?

None.

Is admin.php always in global scope?

Yes, but it is dangerous to assume that these vars might conditionally spill into global scope, or be set at all.

Which globals does admin.php use that were defined elsewhere?

$pagenow

Where were they defined?

vars.php

My patch properly globalizes them (they are indeed globals elsewhere) and properly imports $pagenow from vars.php. wp-admin/includes/plugin.php had to do an isset() check every time it wanted to touch $plugin_page, since the variable might not even exist - I have smoothed that out.

[wonderboymusic]

In 31105:

Properly declare $hook_suffix, $plugin_page, $typenow, and $taxnow as globals in wp-admin/admin.php.

Fixes #30958.

[nacin]

While in practice it probably doesn't matter here, there *is* a difference between isset( $a ) and if ( $a ).

However, this seems like it would break any other check (such as in a plugin) of isset( $hook_suffix ) or isset( $plugin_page ) etc, especially since core was doing it (in API functions, not just where they were maybe declared).

I'd argue that the best approach here would be to specify them as global but keep the initialization as is.

While it's possible — even probable — we break this later down the line, there's no need to do so now. Thoughts?

[wonderboymusic]

ok - I can back out the isset() removals.

global $var ends up being the equiv of $var = null - most of the checks for plugin pages were doing:

• Is it set at all?
• Now let me check it is an array key

isset() would pass for $var = '', which is why truthy/falsely seemed to work instead - but anyone else checking isset() in a plugin might experience side effects since it may no longer be null in some cases.

[wonderboymusic]

In 31106:

After [31105], don't ditch the isset() calls for BC. Declare $page_hook as null so it is initialized for all execution paths but will still fail isset() checks.

Fixes #30958.

[SergeyBiryukov]

Please provide some explanation when reopening a ticket.