WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 10 months ago

#20101 closed defect (bug) (fixed)

Can't use 'show_admin_bar' filter with conditional tags

Reported by: scribu Owned by: nacin
Milestone: 3.6 Priority: normal
Severity: normal Version:
Component: Toolbar Keywords: 3.6-early dev-feedback
Focuses: Cc:

Description

I'm trying to show the admin bar only on the front page:

function scribu_show_admin_bar() {
  return is_front_page();
}

add_filter( 'show_admin_bar', 'scribu_show_admin_bar' );

However, this doesn't work because '_wp_admin_bar_init' is called on 'init', before the main WP_Query is set up.

Attachments (1)

20101.diff (999 bytes) - added by scribu 2 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 scribu2 years ago

  • Component changed from General to Toolbar

comment:2 scribu2 years ago

I also tried this variation:

function scribu_show_admin_bar() {
  return !did_action( 'template_redirect' ) || is_front_page();
}

which almost worked: I got the blank space mentioned in #16249

scribu2 years ago

comment:3 scribu2 years ago

  • Keywords has-patch added

comment:4 nacin19 months ago

  • Milestone changed from Awaiting Review to 3.5

I could go for this.

I'd be curious what will break in doing this. (Surely it will be something...)

comment:5 helenyhou18 months ago

  • Keywords needs-testing punt added

comment:8 scribu18 months ago

I'm afraid this doesn't amend itself well to unit testing, so I could wax poetical about it all day, or we could just give it a soak in trunk and revert it if it breaks a lot of stuff.

comment:9 scribu18 months ago

I'm bluffing; I can't write verbosely to save my life, but it does seem like a pretty low risk fix.

comment:10 helenyhou18 months ago

I personally would be happy enough with you saying you tried running the patch yourself for a little while and didn't see anything weird with any of your fancy themes or complicated plugins like bbPress or BuddyPress :)

comment:11 scribu18 months ago

I just tried it with the latest version of BuddyPress and, interestingly enough, it works.

It also works fine in bbPress, as expected.

comment:12 helenyhou18 months ago

  • Keywords punt removed

I can be reasonable. You can hash it out over actually getting it committed, though.

comment:13 nacin18 months ago

  • Keywords 3.6-early commit added
  • Milestone changed from 3.5 to Future Release

I still have a bad taste in my mouth after #20161, which caused some (limited) grief for plugins, and actually caused enough issues on WordPress.com (because of some toolbar items they add) that they just reverted the change. I do think it is a good change, but let's do this early 3.6. (Seriously — the day we branch, someone hold me to it.)

comment:14 SergeyBiryukov16 months ago

  • Milestone changed from Future Release to 3.6

comment:15 nacin14 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 23512:

Move admin bar initialization from init to template_redirect, so conditional tags may be used in the show_admin_bar filter. props scribu. fixes #20101.

comment:16 ryan12 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This broke some things on wordpress.com. We've reverted it for now. I'll investigate whether what we are doing that triggers the problem is a remotely sane thing to do. :-) Meanwhile, I'll reopen this as a reminder.

comment:17 nacin12 months ago

If I recall correctly, WordPress.com never bothered to move things out of the footer to begin with (after #20161). So there is likely a plugin somewhere that removed the filter you shouldn't remove and added it back. This is what broke, I imagine.

comment:18 ocean9011 months ago

  • Keywords dev-feedback added; has-patch needs-testing commit removed

Status?

comment:19 ocean9010 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.