-
Notifications
You must be signed in to change notification settings - Fork 427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent TGMPA notice showing on WP core update pages. #512
Conversation
*/ | ||
protected function is_core_update_page() { | ||
// Current screen is not always available, most notably on the customizer screen. | ||
if ( function_exists( 'get_current_screen' ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do an inverse check, and return false early.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
9c5843f
to
6bf1947
Compare
Prevents core/plugin/theme updates screens looking like: ![screenshot](http://snag.gy/mIFC6.jpg)
6bf1947
to
ddc1275
Compare
I got push-back from WPCS about not checking the nonce for the I did change the |
It's not our nonce, but if there is a nonce, we should still check it, since we're still wanting to confirm that we're on the plugins / update page, and that an action was posted from within that page. My making an external POST to example.com/wp-admin/plugins.php should not trigger TGMPA into doing anything. We may not be using the value, but we are using the fact the key exists, to make an assumption. Whitelisting is the wrong decision here IMO. |
That's the whole point why I did whitelist. In that case, we don't want to expose TGMPA and therefore shouldn't show the notice. The alternative would be to check the nonce + rest and if ok, not show the notice and if not ok, also not show the notice, so the end result would be exactly the same, just with more code. |
Whitelisting doesn't affect exposure. Using the nonce does. The nonce is the right way to go here. |
@GaryJones Just been looking into this, but there are something like 8 different nonces to check. This is going to be a maintenance nightmare, quite apart from the fact that I stand by my point that it's totally useless to do this as - I repeat-:
|
Prevents core/plugin/theme updates screens looking like this: