Skip to:
Content

BuddyPress.org

Opened 11 years ago

Closed 11 years ago

Last modified 8 years ago

#4803 closed defect (bug) (fixed)

bp_core_load_admin_bar() is called too early (bp_loaded instead of bp_init)

Reported by: johnjamesjacoby's profile johnjamesjacoby Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 1.7.1 Priority: normal
Severity: normal Version: 1.7
Component: Toolbar & Notifications Keywords: dev-feedback has-patch
Cc:

Description

Currently bp_core_load_admin_bar() is hooked to bp_loaded. Since we're doing is_user_logged_in() checks (see #3661), we're invoking the current user (and subsequently the $current_user global) before $wp_roles are loaded.

I propose we move bp_core_load_admin_bar() to bp_init, and also move the function itself into bp-core-adminbar.php next to bp_core_load_admin_bar_css().

Attachments (1)

4803.01.patch (2.0 KB) - added by r-a-y 11 years ago.

Download all attachments as: .zip

Change History (13)

#1 @johnjamesjacoby
11 years ago

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

(In [6762]) Move bp_core_load_admin_bar() to bp_init, and also move the function itself into bp-core-adminbar.php next to bp_core_load_admin_bar_css(). Fixes bug causing current user to be loaded out of order. Fixes #4803. See #3661.

#2 follow-up: @r-a-y
11 years ago

This breaks #4771.

We need to run the function any time before or equal to 'init' at priority 9.

If we want to stick with a BP hook, 'bp_loaded' at a really late priority like 999 would work as well, but I'm not sure if that's too early for the current user checks.

#3 @r-a-y
11 years ago

Friendly ping, JJJ.

#4 @r-a-y
11 years ago

  • Keywords dev-feedback added; commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version set to 1.7

Reopening.

#5 @johnjamesjacoby
11 years ago

Replying to r-a-y:

(In [6724]) Fix 'Show the Toolbar for logged out users' setting.

WP recommends to use show_admin_bar() on 'plugins_loaded':
https://codex.wordpress.org/Function_Reference/show_admin_bar#Notes

It's a race to get our logic in before WordPress core, and after $wp->init(); plugins_loaded is too early because of our is_user_logged_in check. init is too late because WP has already decided for us. I'll look into this soon.

#6 @DJPaul
11 years ago

  • Milestone changed from 1.7 to 1.8

#7 in reply to: ↑ 2 @johnjamesjacoby
11 years ago

Replying to r-a-y:

If we want to stick with a BP hook, 'bp_loaded' at a really late priority like 999 would work as well, but I'm not sure if that's too early for the current user checks.

bp_loaded is too early, as it's still hooked to plugins_loaded. It needs to be early on 'init', which I'd rather not need to do for the entire toolbar loading process either.

Let's extract this functionality out and create a function specifically for this purpose, and hook it early onto init.

@r-a-y
11 years ago

#8 @r-a-y
11 years ago

  • Component changed from Core to Toolbar/BuddyBar
  • Keywords has-patch added
  • Milestone changed from 1.8 to 1.7.1

Let's extract this functionality out and create a function specifically for this purpose, and hook it early onto init.

In 01.patch, I:

  • Chose not to extract the functionality out of bp_core_load_admin_bar() because I feel we would be creating a new function that wouldn't be used outside this context. Feel free to do whatever is best.
  • Moved the hook to init at priority 9.
  • I removed lines 71-77 in the existing code because it is unnecessary since is_admin_bar_showing() already does the same check in wp_admin_bar_render().

Unit tests forthcoming.

#9 @johnjamesjacoby
11 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 6946:

Fix toolbar not showing for logged out users. Props r-a-y. Fixes #4803.

#10 @r-a-y
11 years ago

JJJ, do you mind if this is included in 1.7-branch as well? Lots of support threads for this.

#11 @r-a-y
11 years ago

In 6965:

Fix toolbar not showing for logged out users (1.7-branch).

See #4803.

#12 @DJPaul
8 years ago

  • Component changed from General - Toolbar/BuddyBar to Toolbar & Notifications
Note: See TracTickets for help on using tickets.