WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#19429 closed defect (bug) (wontfix)

Prevent the my-account node from being removed

Reported by: nacin Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.3
Component: Toolbar Keywords:
Focuses: Cc:

Description

my-account contains the only logout link in WordPress. If someone removed it in 3.2 to simplify the admin bar, the user will be pretty much out of luck. At least one plugin in the directory does do this.

We should prevent remove_node( 'my-account' ).

That said, this does not prevent the node from being modified in other ways. add_node() can still replace the node, modifying its parent (or even orphaning it), etc. We just need to prevent outright removal to allow users to still log out.

Attachments (2)

19429.diff (421 bytes) - added by nacin 2 years ago.
19429.2.diff (3.2 KB) - added by nacin 2 years ago.

Download all attachments as: .zip

Change History (12)

nacin2 years ago

comment:1 nacin2 years ago

  • Keywords has-patch added

comment:2 nacin2 years ago

Since the action can just be unhooked, Koop and I have a different idea. On bind, we should scan for a logout link. If one does not exist, we should force one on the far right.

comment:3 koopersmith2 years ago

  • Keywords needs-patch needs-refresh added; has-patch removed

Boom.

comment:4 nacin2 years ago

Code from koopersmith push it onto the end of the the root element. (We raced, he won.) Holding off until the code in #19416 is in.

		// Ensure we have a logout node.
		$root = $this->_get_node( 'root' );
		$node = $this->get_node( 'logout' );
		
		while ( $node && $node->parent ) {
		    if ( $found_root = ( 'root' == $node->parent ) )
		        break;
		    $node = $this->get_node( $node->parent );
		}
		
		if ( empty( $found_root ) ) {
		    $this->add_node( array(
		        'parent' => 'root',
		        'id'     => 'logout',
		        'title'  => __( 'Log Out' ),
		        'href'   => wp_logout_url(),
		    ) );
		    $root->children[] = $this->get_node( 'logout' );
		}

Further thoughts.

1, we'll still want 19429.diff, otherwise a plugin from 3.2 can do some damage pretty easily.

2, since the priorities will be changing, #19425, a remove_filter() that worked in 3.2 for wp_admin_bar_my_account_menu() will not work in 3.3. So we're covered there as well.

Version 0, edited 2 years ago by nacin (next)

comment:5 nacin2 years ago

A _doing_it_wrong() notice should be issued in the case where no logout link is present.

comment:6 nacin2 years ago

  • Keywords has-patch added; needs-patch removed

Has patch, needs refresh.

I am editing comment:4 to include _doing_it_wrong().

comment:7 nacin2 years ago

OR:

Let's just rename 'my-account' to 'account'. We'll have to change the CSS and add it to the back compat array (#19426), but this solves all of our problems.

Sometimes it takes a bit for the obvious answer to hit you.

nacin2 years ago

comment:8 nacin2 years ago

  • Keywords commit added; needs-refresh removed

Renames it to 'account'. We'll also want to add 'my-account' to the back compat array in #19426.

comment:9 nacin2 years ago

  • Keywords 2nd-opinion punt added; commit removed

Per IRC, ryan and westi didn't really like this. I want to chew on it a little more.

comment:10 nacin2 years ago

  • Keywords has-patch 2nd-opinion punt removed
  • Milestone 3.3 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

The only three plugins that tried to modify the my-account menu are wordpress-admin-bar-improved, cmsify, and s2member.

s2member has already been updated to use the 3.3 APIs, and was only modifying the menu, rather than removing it.

wordpress-admin-bar-improved (9,440 downloads) and cmsify (1,948 downloads) use a series of checkboxes to allow menus to be turned on or off. So we're good here.

wordpress-admin-bar-improved claims it is compatible with 3.3-beta3 but still tries to use $wp_admin_bar->menu, which was gone before then. So they have a bit of work to do.

One other thing I noticed was that a plugin by johnbillion, user-switching, adds a node to both my-account and my-account-with-avatar. In 3.3, due to the mapping, there will be two items, rather than one. (We don't check for duplicates.) I think this is not the end of the world, and obviously john can fix it.

Note these searches were run on a plugins directory download that was about 7 weeks old. $wp_admin_bar->menu was removed 6 weeks ago. I'll be re-running the searches once the update is complete, but I don't expect any movement.

Note: See TracTickets for help on using tickets.