Skip to:
Content

Opened 15 months ago

Closed 12 months ago

Last modified 12 months ago

#4803 closed defect (bug) (fixed)

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

Reported by: johnjamesjacoby Owned by: johnjamesjacoby
Milestone: 1.7.1 Priority: normal
Severity: normal Version: 1.7
Component: Toolbar/BuddyBar 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 13 months ago.

Download all attachments as: .zip

Change History (12)

comment:1 johnjamesjacoby15 months 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.

comment:2 follow-up: r-a-y15 months 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.

comment:3 r-a-y15 months ago

Friendly ping, JJJ.

comment:4 r-a-y14 months ago

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

Reopening.

comment:5 johnjamesjacoby14 months 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.

comment:6 DJPaul13 months ago

  • Milestone changed from 1.7 to 1.8

comment:7 in reply to: ↑ 2 johnjamesjacoby13 months 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-y13 months ago

comment:8 r-a-y13 months 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.

comment:9 johnjamesjacoby12 months 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.

comment:10 r-a-y12 months ago

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

comment:11 r-a-y12 months ago

In 6965:

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

See #4803.

Note: See TracTickets for help on using tickets.