Make WordPress Core

Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#16005 closed defect (bug) (fixed)

When trying to remove an item from the admin bar, a blank spot is added in lieu of it

Reported by: sorich87's profile sorich87 Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.1
Component: Administration Keywords: has-patch
Focuses: Cc:

Description

I am trying to remove items from the admin bar (parent or child items) but a blank spot is added in lieu of the removed item. See attached screenshot.

Here is the code I am using:

add_action( 'admin_bar_menu', 'wpss_admin_bar_menu', 100 );

function wpss_admin_bar_menu() {
	global $wp_admin_bar;

	$wp_admin_bar->remove_menu( 'new-post' );
	$wp_admin_bar->remove_menu( 'my-blogs' );
}

Attachments (7)

Selection_039.png (13.6 KB) - added by sorich87 14 years ago.
shows the issue
16005.diff (639 bytes) - added by sorich87 14 years ago.
16005.2.diff (863 bytes) - added by sorich87 14 years ago.
patch rewritten to follow WordPress coding standards
16005.3.diff (637 bytes) - added by sorich87 14 years ago.
oops! error in the previous patch
16005.4.diff (415 bytes) - added by sorich87 14 years ago.
unset menu item
16005.5.diff (693 bytes) - added by SergeyBiryukov 14 years ago.
wp3.1-remove_menu-link-removal-issue.png (21.9 KB) - added by kpdesign 14 years ago.

Download all attachments as: .zip

Change History (21)

#1 @nacin
14 years ago

  • Milestone changed from Awaiting Review to 3.1

@sorich87
14 years ago

shows the issue

@sorich87
14 years ago

#2 @sorich87
14 years ago

  • Keywords has-patch 2nd-opinion added

The attached patch fixes the issue, but I am not sure it is the best way to do it.

The error is due to the fact that when remove_node() is called by remove_menu(), it returns an empty menu item (line 200 wp-includes/class-wp-admin-bar.php): $menu_item = null;.

The attached patch prevent render() from returning anything when the menu item is null.

Another way to fix the issue would be to unset the menu item in remove_node() instead of returning null. I tried: unset($menu->{$menu_item_id}) but it worked only for parent items. I am not sure about how to do the same for childs.

@sorich87
14 years ago

patch rewritten to follow WordPress coding standards

@sorich87
14 years ago

oops! error in the previous patch

@sorich87
14 years ago

unset menu item

#3 @sorich87
14 years ago

  • Keywords 2nd-opinion removed

Forget my previous comment. The unset works. See latest patch.

#4 @nacin
14 years ago

The code around the patch looks like it can be improved.

First, PHP4 issue with the by-reference variable in the foreach. That needs to be removed (and appears useless based on the patch fuzz).

Scrapping the foreach and doing this should be enough.

if ( isset( $menu->$id ) ) {
   unset( $menu->id );
   return true;
}
return false;

#5 @nacin
14 years ago

  • Keywords needs-patch added; has-patch removed

#6 @sorich87
14 years ago

Without the foreach loop, removing child items (e.g. $wp_admin_bar->remove_menu( 'new-post' );) will not work.

#7 @nacin
14 years ago

Yep, saw that. I think the new patch works nicely though.

#8 @sorich87
14 years ago

  • Keywords has-patch added; needs-patch removed

I agree it works nicely.

#9 @ryan
14 years ago

Patches works well here. To clarify, should that be new-content instead of new-post in the example.

#10 @SergeyBiryukov
14 years ago

Without the patches, there's a blank spot when removing either of them.

#11 @ryan
14 years ago

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

(In [17184]) Clean up admin bar node removal. Make it PHP4 safe and prevent display of blank spots. Props sorich87, SergeyBiryukov. fixes #16005

#12 follow-up: @kpdesign
14 years ago

  • Cc kpdesign added

In a convo with Andrew Nacin on Twitter a couple of days ago, I asked if removing child items was not possible (based on comment 6 to this ticket). He said that the final patch took that into consideration and that child items should be able to be removed.

Using the following code to remove the dashboard links in the admin bar in Multisite:

function remove_admin_bar_dashboard_link() {
	global $current_user, $wp_admin_bar;

	if (!current_user_can('edit_posts') && is_admin_bar_showing()) {
		$wp_admin_bar->remove_menu('dashboard');
	}
}

add_action('admin_bar_menu', 'remove_admin_bar_dashboard_link', 100);

This results in the dashboard link in the My Account menu being removed, but it does not remove the My Sites/[Site Name]/Dashboard links (see attached screenshot).

My understanding of the remove_menu() function is that it looks for the slug for the menu item. Wouldn't the slug for all of the Dashboard links in the admin bar be 'dashboard' and thus removable using the above code?

#13 in reply to: ↑ 12 ; follow-up: @sorich87
14 years ago

Replying to kpdesign:

My understanding of the remove_menu() function is that it looks for the slug for the menu item. Wouldn't the slug for all of the Dashboard links in the admin bar be 'dashboard' and thus removable using the above code?

The ID of these items is not "dashboard" but "blog-{$blog->userblog_id}-d". See /wp-includes/admin-bar.php, line 137.

#14 in reply to: ↑ 13 @kpdesign
14 years ago

Replying to sorich87:

The ID of these items is not "dashboard" but "blog-{$blog->userblog_id}-d". See /wp-includes/admin-bar.php, line 137.


Are you referring to /wp-includes/admin-bar.php, line 122?

	$wp_admin_bar->add_menu( array( 'parent' => 'blog-' . $blog->userblog_id, 'id' => 'blog-' . $blog->userblog_id . '-d', 'title' => __( 'Dashboard' ), 'href' => get_admin_url($blog->userblog_id) ) );

I tried the following code in my function to remove those menu links, based on the id in the above code, without any success - those links still show for a subscriber:

	$wp_admin_bar->remove_menu('blog-'.$blog->userblog_id.'-d');
Note: See TracTickets for help on using tickets.