Skip to:
Content

BuddyPress.org

Opened 10 years ago

Closed 10 years ago

Last modified 5 years ago

#3645 closed defect (bug) (worksforme)

WP Admin Bar doesn't honour settings

Reported by: r-a-y Owned by:
Milestone: Priority: normal
Severity: normal Version: 1.5
Component: Toolbar & Notifications Keywords:
Cc:

Description

In bp_core_load_admin_bar(), BP doesn't check to see if a user is logged in before checks are done for the user's admin bar settings.

Also, when checking the user's admin bar settings, BP doesn't do an exact boolean check.

This caused the WP Admin Bar to not display for non-logged-in users even though "Hide admin bar for logged out users?" was set to "No".

Attachments (2)

3645.01.patch (937 bytes) - added by r-a-y 10 years ago.
3645.02.patch (1.0 KB) - added by r-a-y 10 years ago.

Download all attachments as: .zip

Change History (17)

@r-a-y
10 years ago

#1 @boonebgorges
10 years ago

  • Milestone changed from Awaiting Review to 1.5.1

Tempted to mark this ticket invalid just because you spelled it 'honour' ;)

#2 @boonebgorges
10 years ago

Actually, I am having a hard time reproducing the issue. The WP admin bar isn't showing for non-logged-in users, regardless of my BP settings. Is there another settings somewhere that I'm missing?

#3 @r-a-y
10 years ago

Forgot to say that I also set define( 'BP_USE_WP_ADMIN_BAR', true ); in bp-custom.php.

Re: "honour" - Forgot I was catering to a US crowd ;) At least Paul will be pleased!

#4 @boonebgorges
10 years ago

Forgot to say that I also set define( 'BP_USE_WP_ADMIN_BAR', true ); in bp-custom.php.

Same here. Logged-in users are not seeing the admin bar. WP 3.2.1, BP trunk r5233. In fact, it's just the opposite - setting "Hide admin bar for logged out users" to "No" doesn't change anything.

However, 3645.01.patch *does* fix this latter problem. On this basis, I'm going to assume that there's another setting somewhere that is different between my installation and r-a-y's which is causing my inability to reproduce the original problem, but that the patch works properly. Going to hold off on committing the change until I can get a sanity check on this from someone else.

#5 @boonebgorges
10 years ago

  • Keywords commit added

@r-a-y
10 years ago

#6 @r-a-y
10 years ago

  • Keywords needs-testing added; commit removed

After looking into this a little further, it looks like bp_get_admin_bar_pref() wasn't mirroring the WP equivalent of _get_admin_bar_pref() fully.

After mirroring the function and wrapping an is_user_logged_in() check around the bp_get_admin_bar_pref() conditional block, this appears to work now.

Please give it a test.

---

Perhaps there's a reason why BP is using a wrapper function for _get_admin_bar_pref(). Is it something to do with the is_multisite() check? If so, remove that from the patch.

Last edited 10 years ago by r-a-y (previous) (diff)

#7 @boonebgorges
10 years ago

  • Keywords needs-refresh added
  • Milestone changed from 1.5.2 to 1.5.3

r-a-y, sorry this ticket has languished. Can I ask you to refresh to current 1.5 branch and WP 3.3, as it's possible that a relevant piece of the WP toolbar api has changed?

#8 @cnorris23
10 years ago

It should be noted that the original bp_get_admin_bar_pref() did correctly mirror _get_admin_bar_pref(). The function bp_get_admin_bar_pref() was based off WP 3.2 code which differed from WP 3.1 _get_admin_bar_pref().

#9 @boonebgorges
10 years ago

  • Milestone changed from 1.5.3 to 1.5.4

#10 @r-a-y
10 years ago

This might not be relevant any more since in WP 3.3, the WP Toolbar only offers you one option, whereas in WP 3.2, there were two -- 1) when viewing site and 2) in dashboard -- under "Users > My Profile".

Last edited 10 years ago by r-a-y (previous) (diff)

#11 @johnjamesjacoby
10 years ago

  • Milestone changed from 1.5.4 to 1.6

Moving to 1.6 to get 1.5.4 out.

#12 @DJPaul
10 years ago

  • Keywords reporter-feedback added

Just been testing this against 3.3.1 / 1.6-trunk. Can't see anything incorrect. Can someone else check and confirm if we can close this ticket?

#13 @r-a-y
10 years ago

  • Keywords reporter-feedback removed

This issue was only relevant in WP 3.2. Feel free to close.

#14 @johnjamesjacoby
10 years ago

  • Component changed from Core to Toolbar/BuddyBar
  • Keywords has-patch needs-testing needs-refresh removed
  • Milestone 1.6 deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Thanks r-a-y. Closing.

#15 @DJPaul
5 years ago

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