Opened 13 years ago
Closed 13 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)
Change History (12)
#4
@
13 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' ); $logout = $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 ) ) { if ( $logout ) $message = 'The <code>logout</code> admin bar node was orphaned. WordPress has attached it so users are able to log out.'; else $message = 'The <code>logout</code> admin bar node was removed. WordPress has added it so users are able to log out.'; _doing_it_wrong( __CLASS__, '3.3', $message . ' (The <code>my-account</code> menu should not be removed.)' ); $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.
#5
@
13 years ago
A _doing_it_wrong() notice should be issued in the case where no logout link is present.
#6
@
13 years ago
- Keywords has-patch added; needs-patch removed
Has patch, needs refresh.
I am editing comment:4 to include _doing_it_wrong().
#7
@
13 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.
#8
@
13 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.
#9
@
13 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.
#10
@
13 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.
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.