Calling show_admin_bar( false ) should dehook toolbar entirely

[danielbachhuber]

Because I'm only hiding the toolbar in certain contexts, I'm calling show_admin_bar( false ) right before get_header(). #16249 was closed suggesting the function should only be called on init. #21746 suggests calling after plugins_loaded However, I think calling it after the query is set and the template is picked is a valid use case.

As such, I'd propose calling show_admin_bar( false ) should run remove_action('wp_head', '_admin_bar_bump_cb'); and similar setup callbacks.

[paulschreiber]

What other callbacks count as "and similar setup callbacks"?

[sabernhardt]

The patch still applies, though it could use a ​Yoda condition on the check.

Also, it would help if someone with deeper knowledge of the admin bar (and themes) agrees this is the correct way to do this. I have used the show_admin_bar filter before, and returning false with that removes the style tag. But I have not used show_admin_bar( false ) to know how that works.

[sabernhardt]

It's a bit late for 5.8. Moving to future release.

[audrasjb]

Patch refreshed against trunk.
I tested the patch and I can confirm it unhook entirely the admin bar and its assets.

Adding commit keyword and moving to milestone 5.9.

[audrasjb]

Toolbar: unhook the admin bar when a falsey value is used on show_admin_bar()

[peterwilsoncc]

Removing the commit keyword as it needs a little extra work:

• admin-bar.css is still enqueued in the header
• admin-bar.js is still enqueued in the footer
• <style media="print">#wpadminbar { display:none; }</style> is still output in the header (harmless but not ideal)
• it's possible for themes & plugins to set a custom callback add_theme_support( 'admin-bar', array( 'callback' => '28569_admin_bar_cb' ) );

Testing notes: added show_admin_bar( false ); to my theme's header.php prior to the wp_head() function call.

I'll reach out to a Jetpack team member to ensure they're aware of this change coming so they can check for compatibility with the WordPress.com bar. I'd rather not surprise them and break 5 million sites.

[obenland]

Let's also make sure that it works as expected when set to true again after.

<?php
show_admin_bar( false );
$this->assertFalse( has_action( 'wp_head', '_admin_bar_bump_cb' ) );

show_admin_bar( true );
$this->assertEquals( 10, has_action( 'wp_head', '_admin_bar_bump_cb' ) );

[obenland]

Looking at this a little bit more, the toolbar can also be toggled on/off through the show_admin_bar filter, which has no connection to the function.

[chaion07]

Hi @afercia! Thank you for reporting this. This ticket was discussed during a recent ​Bug-scrub session. Based on the feedback received we're updating the ticket. It would be great to have a patch too. Thanks!

Props to @audrasjb & @costdev

[sabernhardt]

Moving to Future Release until the patch is refreshed, with unit tests.