WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#29251 closed defect (bug) (fixed)

Redirect URL of "Edit <term>" from frontend is wrong.

Reported by: DzeryCZ Owned by: boonebgorges
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.9.2
Component: Taxonomy Keywords: has-patch
Focuses: administration Cc:

Description

Hello,

When you're on frontend in some term of custom taxonomy and click on "Edit <term>" in top admin menu bar, you'll be redirect to edit of that taxonomy, but in left menu you're still in "posts" not in custom post type.

I see in /wp-includes/admin-bar.php on line 537 this:

'href' => get_edit_term_link( $current_object->term_id, $current_object->taxonomy)

but I think, that there should be something like this:

'href' => get_edit_term_link( $current_object->term_id, $current_object->taxonomy,get_post_type())

One picture is worth a thousand words:
http://support.jawtemplates.com/goodstore/web/wp-content/uploads/2014/08/WP_CUSTOM_POSTS_BUG.png

Attachments (2)

29251.patch (493 bytes) - added by juliobox 6 years ago.
More correct link, but can we do better?
29251.2.patch (2.7 KB) - added by boonebgorges 5 years ago.

Download all attachments as: .zip

Change History (13)

#1 @juliobox
6 years ago

Hello

What if this taxonomy is associated with 2 CPTs ?
Actually, no CPTs info is given so the "post" tab is open. And the thing works with "categories" because this tax is associated with posts, already opened.
But now with another CPT, if a taxo is associated with 2, do we have to open the 1st CPT found, even if this is not right, neither wrong.
What do you think ?

@juliobox
6 years ago

More correct link, but can we do better?

#2 @juliobox
6 years ago

  • Keywords has-patch added

#3 @DzeryCZ
6 years ago

Hello,

Yes. If is one taxonomy associated with two CPTs, so there is dilemma.
It'll redirect you to CPT submenu of post type of the first post in viewed loop.
But I think that is better to have opened menu of the first or the second CPT, rather then default "Posts"

#4 @juliobox
6 years ago

i guess also, so now, we got a correct CPT menu opened with the taxonomy in "bold" style.
Seams correct for me.

#5 @joedolson
5 years ago

  • Focuses accessibility removed

#6 @boonebgorges
5 years ago

  • Milestone changed from Awaiting Review to 4.2

Good catch. I don't think there's much we can do in the case of multiple object types - there's no real way to tell which one is "meant", so I think the best we can do is to grab the first one.

I'm going to add a little additional logic to ensure that there are no errors in case the taxonomy archive has no items (so $post may not be set), or in case the taxonomy is not associated with any object_type.

#7 @boonebgorges
5 years ago

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

In 31218:

In get_edit_term_link(), default to a valid $object_type.

The $object_type param is used to set the 'post_type' query var, which
determines the post type menu that will be expanded when clicking through to
the term edit page. Not all taxonomies are associated with Posts, so it makes
sense to default to a post_type that the taxonomy is actually associated with.

Props DzeryCZ, juliobox.
Fixes #29251.

#8 follow-up: @nacin
5 years ago

@boonebgorges: Minor point, I wonder how this previously behaved when an object type is not a post type. (Say, a user or a link.)

We don't support a taxonomy to have multiple object types unless they are all post types (all or none), so there isn't a concern of ambiguity, just perhaps something with a user taxonomy...

#9 @nacin
5 years ago

We could filter (array_filter( $tax->object_type, 'post_type_exists' ), or probably better, check post_type_exists( reset( $tax->object_type ) ). But if it's not, well, then I'm not sure what the behavior was or would be now, but I reckon we shouldn't set a &post_type= QV.

(also: else if => elseif)

#10 in reply to: ↑ 8 ; follow-up: @boonebgorges
5 years ago

Replying to nacin:

We don't support a taxonomy to have multiple object types unless they are all post types (all or none), so there isn't a concern of ambiguity, just perhaps something with a user taxonomy...

By "we don't support" I assume you mean that plugin authors need to do their own 'update_count_callback' logic and stuff like that. I don't see anywhere where we enforce all-or-none.

@boonebgorges: Minor point, I wonder how this previously behaved when an object type is not a post type. (Say, a user or a link.)

Yeah, I'd overlooked this. A more general fix is 29251.2.patch - your suggestions assume the all-or-none enforcement you suggest above, and will miss a post-type object_type if it's not the first in the list. As for whether this needs to be fixed right now: The clause in question is only called when $object_type is not passed to get_edit_term_link(). In core, this only happens when generating the admin bar link, and this admin bar link only appears in term archives, which aren't natively supported for objects other than post types. So it'd only be an issue in plugins.

And even there, if 'post_type=foo' is included in the URL, it'll be ignored if 'foo' is not a post type, and the 'Posts' menu will be highlighted on edit-tags.php. This is the same as what happens when no 'post_type' qv is provided, which means that the only change in behavior between 4.1 and 4.2 is the appearance of the QV in the URL.

My suggestion is to fix this for 4.3 as part of a separate ticket, but 4.2 is fine too if you think it important and you've reviewed the patch.

#11 in reply to: ↑ 10 @nacin
5 years ago

Replying to boonebgorges:

By "we don't support" I assume you mean that plugin authors need to do their own 'update_count_callback' logic and stuff like that. I don't see anywhere where we enforce all-or-none.

We don't support as in the object IDs would clash in term_relationships. :-)

Note: See TracTickets for help on using tickets.