![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 12 years ago
Last modified 4 days ago
#28569 reviewing enhancement
Calling show_admin_bar( false ) should dehook toolbar entirely
| Reported by: | danielbachhuber | Owned by: | davidbaumwald |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.1 |
| Component: | Toolbar | Version: | |
| Severity: | normal | Keywords: | has-patch needs-unit-tests |
| Cc: | Focuses: |
Description
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.
Attachments (2)
Change History (19)
This ticket was mentioned in Slack in #core by sabernhardt. View the logs.
5 years ago
#6
@
5 years ago
- Milestone → 5.8
- Owner changed from to
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/or 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.
This ticket was mentioned in Slack in #core by sabernhardt. View the logs.
5 years ago
#8
@
5 years ago
- Keywords needs-refresh added; good-first-bug removed
- Milestone 5.8 → Future Release
It's a bit late for 5.8. Moving to future release.
#9
@
5 years ago
- Keywords commit added; needs-refresh removed
- Milestone Future Release → 5.9
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.
#10
@
5 years ago
- Keywords needs-refresh added; commit removed
Removing the commit keyword as it needs a little extra work:
admin-bar.cssis still enqueued in the headeradmin-bar.jsis 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.
#11
@
5 years ago
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' ) );
#12
@
5 years ago
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.
This ticket was mentioned in Slack in #core by chaion07. View the logs.
5 years ago
#14
@
5 years ago
- Keywords needs-unit-tests added
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
#15
@
5 years ago
- Milestone 5.9 → Future Release
Moving to Future Release until the patch is refreshed, with unit tests.
This ticket was mentioned in PR #12402 on WordPress/wordpress-develop by @sainathpoojary.
8 days ago
#16
- Keywords needs-refresh removed
Trac ticket: #28569
What other callbacks count as "and similar setup callbacks"?