Site-kit-wp: Trying to get property 'base' of non-object in .../Core/Admin_Bar/Admin_Bar.php on line 147

[RealBenihime]

Bug Description

So far I haven't picked up any issues with operations, however, the error does show up on the frontend only of the site.

Notice: Trying to get property 'base' of non-object in /.../wp-content/plugins/google-site-kit/includes/Core/Admin_Bar/Admin_Bar.php on line 147

Steps to reproduce

1. Install and Activate Google Site Kit Plugin
2. Complete Setup Plugin
3. Navigate to frontend (Debug must be set to True on wp-config.php)

Screenshots

Additional Context

• PHP Version: 7.2.19-0ubuntu0.18.04.1
• OS: [e.g. iOS] (Client)Windows 10 | (Server)Linux
• Browser: [e.g. chrome, safari]Chrome
• Plugin Version: [e.g. 22] 1.0.0-beta.1.0.3
• Device: [e.g. iPhone6] Dell Inspiron 15 7000

---

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

• No PHP error notice should appear from the Site Kit admin bar when browsing the frontend.
• The plugin should not rely on a "current screen" being available just because the function is available.

Implementation Brief

The logic for determining if the Admin Bar should display for a particular screen (i.e. wp-admin) is convoluted and not easy to reason about. Ensuring that a WP_Screen object is available removes the need to check for the availability of specific properties as well.

• Extract this logic from \Google\Site_Kit\Core\Admin_Bar\Admin_Bar::is_active to a new private is_admin_post_screen method on Admin_Bar

        if ( ! is_user_logged_in() || ! is_admin_bar_showing() ) {
            return false;
        }

        // condition was here

        if ( $this->is_admin_post_screen() ) {
            $post = get_post();
        } else {
            $post = get_queried_object();
        }

private function is_admin_post_screen() {
    $current_screen = function_exists( 'get_current_screen' ) ? get_current_screen() : false;

    // No screen context available
    if ( ! $current_screen instanceof \WP_Screen ) {
        return false;
    }

    // Only show for post screens
    if ( 'post' !== $current_screen->base ) {
        return false;
    }

    // Don't show for new post screen
    if ( 'add' === $current_screen->action ) {
        return false;
    }

    return true;
}

Changelog entry

• Fix potential PHP notice when no current WordPress admin screen is present.

[swissspidy]

Thanks for your report!

Looks like there is a missing $current_screen === null check here:

https://github.com/google/site-kit-wp/blob/a0e6e72ecac55ac8adf44f7e16b2a8a32fd4a417/includes/Core/Admin_Bar/Admin_Bar.php#L143-L150

[RealBenihime]

Thanks @swissspidy

I've added the null check and returned false if $current_screen is null, that did the trick.
But I think I'll keep this ticket open until the fix is applied in the next build.

[swissspidy]

Yeah, please don't close :-) The intention was not to give you a hint for a fix, but to fix this properly in the plugin.

[ghost]

Hi,
Just wanted to add that I'm seeing the same.

[aaemnnosttv]

Rather than checking for various falsey values we should instead check that $current_screen is indeed an instance of WP_Screen.

[felixarntz]

@aaemnnosttv IB looks good, with two recommendations:

1. We could rename is_active_for_current_screen() to is_admin_post_screen(), because that's what it checks.
2. We can probably remove the is_admin() check before calling is_active_for_current_screen() / is_admin_post_screen() because the function won't exist outside of is_admin() anyway.

[aaemnnosttv]

IB updated with your recommendations @felixarntz

[tofumatt]

I'm trying to QA this issue but I'm not clear on how to get the Site Kit Admin bar to display on the frontend of the site. Do I need a certain set of permissions for my connected Site Kit site to see it? I'm used filtered data from ElasticPress.io and it doesn't seem to appear...

[tofumatt]

I managed to get the admin bar to appear, though I couldn't reproduce the issue by going to the frontend of the site as either an admin, editor, or anonymous user.

I don't see the error in the current develop branch, but then I can't reproduce it either (using 709d61bad5727614b9cf44bb6abe14ec8e4a00eb, which is before the fix from what I can tell) so I can't confidently mark the issue as fixed. Could you provide more explicit steps to reproduce or a screenshot of the error so I can see what I should be looking for?

[swissspidy]

I assume it was some conflict with another plugin

[tofumatt]

What was a conflict? The lack of error I was seeing or the cause of the initial error in the bug report?

I'm not sure how to QA the issue if it's the latter, because I can't verify it happening to tell if it was fixed 😄

[swissspidy]

The latter.

@RealBenihime maybe you can shed some light on this? Which other plugins are you using?

[tofumatt]

I'm going to un-assign myself from now as I can't reproduce the issue, but once there's more info I'm happy to hop back on the issue 😄

[RealBenihime]

@swissspidy Apparently it is a plugin conflict and I narrowed it down to this plugin (WP File Manager) https://wordpress.org/plugins/wp-file-manager I've confirmed it on 2 sites.

[swissspidy]

Awesome, thanks for checking!

@tofumatt Can you try it with the WP File Manager plugin installed?

[circlecube]

I've tried to reproduce this error without the plugin to no success. I also tried with the plugin installed and still no success. I tried with all the latest default themes (17 18 & 19). Any other specific instruction to reproduce @RealBenihime?

[RealBenihime]

@circlecube I couldn't think of any other way to do this but here is a YT video of a screen recording reproducing the error https://youtu.be/bSkwHkbwkew and attached is the system info that was generated during the recording
system-info-cva.ukzn.ac.za-28-08-2019.txt

[swissspidy]

@RealBenihime Here's a current build of the plugin that includes this bugfix:

google-site-kit.zip

Could you please give it a try and let us know if it resolves the issue?

[RealBenihime]

@swissspidy It does resolve the issue BUT, it also removed the stats on the Adminbar, only thing displayed now is the "Stats for: [page_name]" and "More Details" link

[aaemnnosttv]

@swissspidy @felixarntz The only thing we changed related to this issue specifically was whether or not the Site Kit admin bar item is displayed, where the screen object was accessed that raised the notice. It may be possible a change was made elsewhere that introduced a regression related to what is displayed but that should likely be a separate issue.

[circlecube]

I was able to reproduce this issue with current develop branch. Only when I also installed the Elementor plugin though. It only occurs if you have both Elementor AND WP File Manager plugin installed and active along with Site Kit. The Site Kit dropdown in the menu still functions as expected, but at the top of the page I see the notice:
Notice: Trying to get property 'base' of non-object in /wp-content/plugins/google-site-kit/includes/Core/Admin_Bar/Admin_Bar.php on line 147

[circlecube]

After testing the code from this ticket I no longer see the error, so this passes QA. (earlier comment was just that I had successfully reproduced the error notice)