Make WordPress Core

Opened 17 years ago

Closed 17 years ago

Last modified 33 hours ago

#12270 closed defect (bug) (fixed)

Incorrect menu active for edit custom taxonomy form

Reported by: jfarthing84 Owned by:
Priority: normal Milestone: 3.0
Component: Administration Version: 3.0
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

When using edit-tag-form.php for a custom taxonomy, the correct menu is not highlighted. This is because the variables $parent_file and $submenu_file are not set or not correctly set. The "Edit" links within the tag row actions will need to be updated to include "post_type" as it is needed to point to the correct parent_file and submenu_file for the custom taxonomy menu.

Change History (10)

#1 @scribu
17 years ago

  • Milestone Unassigned → 3.0

#2 @dd32
17 years ago

When using edit-tag-form.php for a custom taxonomy, the correct menu is not highlighted. This is because the variables $parent_file and $submenu_file are not set or not correctly set.

Thats worked for me in core for quite awhile now. See #11838 which is related.

The "Edit" links within the tag row actions will need to be updated to include "post_type" as it is needed to point to the correct parent_file and submenu_file for the custom taxonomy menu.

Thats still a problem AFAIK, and needs to be updated as you suggest

#3 @jfarthing84
17 years ago

I'm not sure you understood. Let's say I have a custom post type "movie". Within that product I have a few taxonomies: "actor", "director", "writer", etc.

Now, when I visit "edit-tags.php?taxonomy=actor&post_type=movie" (which is the default menu created by WP), the correct menu item is highlighted ("Movies") along with the correct sub-menu ("Actors"). However, click the edit link for a specific term (i.e. "edit-tags.php?action=edit&taxonomy=actor" and then the "Posts" menu is highlighted with no submenu highlighted.

This is due to no post_type being passed with the edit link, because the post_type is included in the key for the submenu. Same with the menu.

#4 @jfarthing84
17 years ago

if ( empty($post_type) || !in_array( $post_type, get_post_types( array('show_ui' => true) ) ) )
	$post_type = 'post';

if ( 'post' != $post_type ) {
	$parent_file = "edit.php?post_type=$post_type";
	$submenu_file = "edit-tags.php?taxonomy=$taxonomy&post_type=$post_type";
} else {
	$parent_file = 'edit.php';
	$submenu_file = "edit-tags.php?taxonomy=$taxonomy";
}

Because of that, when editing a term, the post_type is always post because the post_type is not passed via the query. So, $parent_file will always evaluate to 'edit.php' and $submenu_file to 'edit-tags.php?taxonomy=$taxonomy', thus highlighting the incorrect menus.

#5 @dd32
17 years ago

Ah, Now thats clearer. Yes, When editing a term, Its defaulting to the post menu.

#6 @dd32
17 years ago

(In [13213]) Fix $post_type handling/passing in the Taxonomy edit links. Fix the Tag selection query var for edit.php in the Posts links. See #12270, See #11838

#7 @dd32
17 years ago

That commit should fix it. Re-open if you've still got any issues.

#8 @nacin
17 years ago

  • Resolution → fixed
  • Status new → closed

This ticket was mentioned in ​PR #13453 on ​WordPress/wordpress-develop by ​@fushar.


5 weeks ago
#9

  • Keywords has-patch has-unit-tests added; needs-patch removed

## What

Makes public a tri-state for icons, so an icon can ship to Core and be registered server-side without becoming selectable in the editor's icon picker.

public in the Gutenberg manifest shipped to Core / in the core collection REST API + Icon block wp_get_icon()
omitted no — stays in the JS library no no
true yes yes yes
false yes no yes

## Why

Core is replacing the admin bar and admin menu dashicons with @wordpress/icons (#12270). That needs roughly a dozen icons registered server-side, but icons like dashboard or plugins should not be offered as post content. Agreed by @t-hamano, @tyxla and @mcsf on WordPress/gutenberg#79451.

Core needs this before a synced manifest can carry the flag. Once Gutenberg marks those icons, the generated icon-library-manifest.php will contain 'public' => false. WP_Icons_Registry::register() rejects the *entire* registration on any unknown property, so without this patch every flagged icon would silently fail to register on a plugin-less WordPress.

## Changes

  • WP_Icons_Registry::register() accepts public and rejects a non-boolean value with _doing_it_wrong(). The value is not normalised — an absent property already means public, and every read uses ?? true.
  • _wp_register_default_icons() forwards public only when the manifest carries it. The manifest omits the key for public icons, and forwarding it unconditionally would send an unknown property to registries that predate it.
  • WP_REST_Icons_Controller::get_items() skips non-public icons, and get_icon() reports them as not found — otherwise /wp/v2/icons/<collection>/<name> would trivially bypass the collection filter.

public is deliberately not added to the REST schema: a non-public icon never appears in a response, so there is nothing for the field to describe.

## Testing instructions

npm run test:php -- --group icons

New coverage:

  • WP_Icons_Registry::register() returns false for a non-boolean public.
  • A non-public icon is absent from GET /wp/v2/icons while a public one in the same collection is present.
  • GET /wp/v2/icons/<collection>/<name> for a non-public icon returns 404 rest_icon_not_found, and wp_get_icon() still returns its markup — the assertion that proves "registered but hidden" rather than merely "absent".

## Related

  • Gutenberg counterpart: WordPress/gutenberg#82634
  • Consumer: #12270 (admin bar / admin menu icons)

---

⚠️ Draft until a Trac ticket exists. The new tests carry @ticket TODO placeholders that need to be replaced with the real number.

🤖 Generated with ​Claude Code

This ticket was mentioned in ​PR #14208 on ​WordPress/wordpress-develop by ​@fushar.


33 hours ago
#10

What?

Backports WordPress/gutenberg#84552. Ships a generated PHP file with the SVG markup of the core-admin icons, and makes the icons registry use it.

Why?

#12270 replaces the Dashicons in the admin menu and admin bar with core-admin/* icons. Every admin page then reads about 20 SVG files from disk and runs wp_kses() on each of them, a cost the CSS-based Dashicons never had. This was ​raised in review.

These icons ship with core, so their markup is trusted and only changes with a release. Inlining it removes both the file reads and the sanitization.

How?

  • src/wp-includes/assets/icon-library-inlined-icons.php maps each core-admin/* icon name to its SVG markup. It is generated in Gutenberg and copied by a new copy:icon-library-inlined-icons step in build:gutenberg, next to the icon manifest.
  • WP_Icons_Registry::get_content() returns the inlined markup when the icon name is in that file and its file_path is inside wp-includes/images/icon-library/. An icon registered under the same name from another location still has its file read and sanitized.

The inlined markup is the file content with the whitespace between tags removed. Compared to the sanitized output it keeps viewBox in its original casing; wp_kses() lowercases it.

In a local Docker environment without opcache, on top of #12270, this takes the first render of the 20 admin icons from about 6 ms to about 1 ms. This is a micro-benchmark of the icon calls, not a page load profile.

Testing Instructions

Run npm run test:php -- --group icons. The new tests check that every inlined icon exists in the manifest, that its markup is equivalent to its sanitized file, and that the registry serves it.

Draft notes:

  • The pinned Gutenberg build does not contain inlined-icons.php yet, so the committed file was produced by running the Gutenberg generator against the currently pinned icons. Until the pin moves past WordPress/gutenberg#84552, the new copy step has nothing to copy and leaves the file as is.
  • The tests were run locally on top of #12270, not on trunk, so CI is their first run here.

Trac ticket: TBD

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Used for: Implementation, tests and this description; reviewed by me.

Note: See TracTickets for help on using tickets.