Skip to content

Icons API: Add wp_get_icon() to render registered icons as inline SVG #12010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
t-hamano wants to merge 4 commits into
base: trunk
Choose a base branch
from
+166 −0
Open

Icons API: Add wp_get_icon() to render registered icons as inline SVG #12010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d9dc326
Icons API: Add wp_get_icon() to render registered icons as inline SVG.
t-hamano May 29, 2026
54df3d4
Tests: Fix inaccurate docblock referencing nonexistent the_wp_icon().
t-hamano May 29, 2026
7bc1bd8
Merge branch 'trunk' into 64847-wp-icon-function
t-hamano Jun 11, 2026
961087d
Merge branch 'trunk' into 64847-wp-icon-function
t-hamano Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions src/wp-includes/icons.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php
/**
* Icons API: Icon rendering helper functions.
*
* @package WordPress
* @subpackage Icons
* @since 7.1.0
*/

/**
* Returns the SVG markup for a registered icon.
*
* @since 7.1.0
*
* @param string $name The namespaced icon name (e.g. 'core/plus').
* @param array $args {
* Optional. Arguments for the icon. Default empty array.
*
* @type int $size Width and height in pixels. Default 24.
* @type string $class Additional CSS class names. Default empty string.
* @type string $label Accessible label. If provided, the SVG gets
* role="img" and aria-label. If omitted, the SVG
* gets aria-hidden="true". Default empty string.
* }
* @return string SVG markup for the icon, or empty string if not found.
*/
function wp_get_icon( $name, $args = array() ) {

@tyxla tyxla Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker, but we may want to refactor render_block_core_icon() to use this function instead of reinventing some of the same logic.

$icon = WP_Icons_Registry::get_instance()->get_registered_icon( $name );

@mukeshpanchal27 mukeshpanchal27 May 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think better to check if $name is not empty. WDYT?

if ( empty( $name ) ) {
	return;
}

@t-hamano t-hamano May 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since get_registered_icon() returns null if $name is empty, adding an empty check feels redundant.

@sirreal sirreal Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we know about the registered icons? If this is a public API where arbitrary SVG content (that passes KSES checks) may be in the icon, things can get very complicated. One that comes to mind is if the SVG has a viewBox, then a single size argument may be tricky. Are icons required to be square? Is that enforced?

And just to confirm, anything in the registry has passed through KSES, right?

if ( is_null( $icon ) ) {
return '';
}

$svg = $icon['content'];
if ( empty( $svg ) ) {
return '';
}

$args = wp_parse_args(
$args,
array(
'size' => 24,
'class' => '',
'label' => '',
)
);

$processor = new WP_HTML_Tag_Processor( $svg );
if ( ! $processor->next_tag( 'svg' ) ) {
return '';
}

$processor->set_attribute( 'width', (string) $args['size'] );
$processor->set_attribute( 'height', (string) $args['size'] );
Comment on lines +52 to +53

@sirreal sirreal Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we get something other than int, is that possible? The string "10%", or other less sensical values: [ '9', '9' ], "nonsense".

@tyxla tyxla Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, came here to ask some of the same. Also, what about negative integers - should we make sure it's not the case by calling absint() on it or something?

$processor->add_class( 'wp-icon' );

@tyxla tyxla Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How necessary is this class? Are we adding it anywhere else? I don't think we are, and this may mean we're actually producing non-identical output with render_block_core_icon() for example.

If we want to adopt it, we should likely consider adding it to render_block_core_icon() and any other React paths that render icons.


if ( ! empty( $args['class'] ) ) {

@tyxla tyxla Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it fine if we pass multiple classes there (class-1 class-2)? To be precise and use the API correctly, should we explode() those and call add_class() for each of them? If you agree, let's also add a test for that.

$processor->add_class( $args['class'] );
}
Comment on lines +56 to +58

@sirreal sirreal Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a single class, or could it be multiple classes? The ::add_class method expects a single class, and multiple classes should be applied individually.

Something like $processor->add_class( 'wp-icon class2 class3' ) is not how the method is intended to be used.


if ( ! empty( $args['label'] ) ) {
$processor->set_attribute( 'role', 'img' );
$processor->set_attribute( 'aria-label', $args['label'] );
} else {
$processor->set_attribute( 'aria-hidden', 'true' );

@tyxla tyxla Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For decorative icons, should we also disable focusable to match what render_block_core_icon() is doing?

Suggested change
$processor->set_attribute( 'aria-hidden', 'true' );
$processor->set_attribute( 'aria-hidden', 'true' );
$processor->set_attribute( 'focusable', 'false' );

}
Comment on lines +60 to +65

@sirreal sirreal Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider that the svg content may already have these attributes, so if one branch should have role and aria-label but not aria-hidden, and vice-versa for the other branch, then it's a good idea to remove the attributes from the other branch so we don't wind up with:

<svg aria-hidden="true" role="img" aria-label="whoops?" />
Suggested change
if ( ! empty( $args['label'] ) ) {
$processor->set_attribute( 'role', 'img' );
$processor->set_attribute( 'aria-label', $args['label'] );
} else {
$processor->set_attribute( 'aria-hidden', 'true' );
}
if ( ! empty( $args['label'] ) ) {
$processor->set_attribute( 'role', 'img' );
$processor->set_attribute( 'aria-label', $args['label'] );
$processor->remove_attribute( 'aria-hidden' )
} else {
$processor->set_attribute( 'aria-hidden', 'true' );
$processor->remove_attribute( 'role' );
$processor->remove_attribute( 'aria-label' )
}

(Do check me and test this, consider the suggestion pseudo-code)


return $processor->get_updated_html();

@tyxla tyxla Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify, are we intentional about not adding a filter here?

}
1 change: 1 addition & 0 deletions src/wp-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@
require ABSPATH . WPINC . '/class-wp-connector-registry.php';
require ABSPATH . WPINC . '/connectors.php';
require ABSPATH . WPINC . '/class-wp-icons-registry.php';
require ABSPATH . WPINC . '/icons.php';
require ABSPATH . WPINC . '/widgets.php';
require ABSPATH . WPINC . '/class-wp-widget.php';
require ABSPATH . WPINC . '/class-wp-widget-factory.php';
Expand Down
97 changes: 97 additions & 0 deletions tests/phpunit/tests/icons/wpGetIcon.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
<?php
/**
* Unit tests covering the wp_get_icon() function.
*
* @package WordPress
* @subpackage Icons
* @since 7.1.0
*
* @group icons
*
* @covers ::wp_get_icon
*/
class Tests_Icons_WpGetIcon extends WP_UnitTestCase {

/**
* @ticket 64847
*/
public function test_wp_get_icon_returns_svg_for_known_icon() {
$output = wp_get_icon( 'core/plus' );
$this->assertStringStartsWith( '<svg ', $output );
$this->assertStringContainsString( '</svg>', $output );
}

/**
* @ticket 64847
*/
public function test_wp_get_icon_returns_empty_string_for_unknown_icon() {
$output = wp_get_icon( 'this-icon-does-not-exist' );
$this->assertSame( '', $output );
}

/**
* @ticket 64847
*/
public function test_wp_get_icon_default_attributes() {
$output = wp_get_icon( 'core/plus' );
// WP_HTML_Tag_Processor lowercases attribute names.
$this->assertStringContainsString( 'viewbox="0 0 24 24"', $output );
$this->assertStringContainsString( 'width="24"', $output );
$this->assertStringContainsString( 'height="24"', $output );
$this->assertStringContainsString( 'class="wp-icon"', $output );
$this->assertStringContainsString( 'aria-hidden="true"', $output );
}

/**
* @ticket 64847
*/
public function test_wp_get_icon_custom_size() {
$output = wp_get_icon( 'core/plus', array( 'size' => 32 ) );
$this->assertStringContainsString( 'width="32"', $output );
$this->assertStringContainsString( 'height="32"', $output );
}

/**
* @ticket 64847
*/
public function test_wp_get_icon_custom_class() {
$output = wp_get_icon( 'core/plus', array( 'class' => 'my-button-icon' ) );
$this->assertStringContainsString( 'class="wp-icon my-button-icon"', $output );
}

/**
* @ticket 64847
*/
public function test_wp_get_icon_with_label() {
$output = wp_get_icon( 'core/plus', array( 'label' => 'Add item' ) );
$this->assertStringContainsString( 'role="img"', $output );
$this->assertStringContainsString( 'aria-label="Add item"', $output );
$this->assertStringNotContainsString( 'aria-hidden', $output );
}

/**
* @ticket 64847
*/
public function test_wp_get_icon_without_label_is_hidden() {
$output = wp_get_icon( 'core/plus' );
$this->assertStringContainsString( 'aria-hidden="true"', $output );
$this->assertStringNotContainsString( 'role="img"', $output );
$this->assertStringNotContainsString( 'aria-label', $output );
}

/**
* @ticket 64847
*/
public function test_wp_get_icon_contains_svg_content() {
$output = wp_get_icon( 'core/plus' );
$this->assertStringContainsString( '<path ', $output );
}

/**
* @ticket 64847
*/
public function test_wp_get_icon_escapes_attributes() {
$output = wp_get_icon( 'core/plus', array( 'class' => '"><script>alert(1)</script>' ) );

@tyxla tyxla Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also test escaping in label and aria-label and any other user-controller attribute?

$this->assertStringNotContainsString( '<script>', $output );

@tyxla tyxla Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the expected form here? &lt;script&gt;? Might make sense to add some more assertions or just change to a positive assertion that matches what exactly we expect.

@sirreal sirreal Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably add some more assertion like assertEqualHTML for things like assertHTMLContainsNoTags( [ 'script' ], $html ) or similar.

For this case, it may be a good idea to create a tag processor with something like this, what we care about is not having a SCRIPT tag:

$p = new WP_HTML_Tag_Processor('<svg><script></script>');
while ( $p->next_token() ) {
  var_dump( $p->get_token_name() );
  if ( 'SCRIPT' === $p->get_token_name() ) {
    echo 'TEST FAILURE';
    throw new Error('bad bad bad');
  }
}
// assert $p->paused_at_incomplete_token() === false;

Or it may be better to just assert that that class was set:

$p = new WP_HTML_Tag_Processor('<hax0r>');
$p->next_tag();
$p->add_class('"><script>alert(1)</script>');
$malicious = $p->get_updated_html();

$p = new WP_HTML_Tag_Processor($malicious);
$p->next_tag();
assert( $p->has_class( '"><script>alert(1)</script>' ) );
echo 'test passed';

}
}
Loading