Skip to content

Icons: Prevent unregistering core-admin icons (Trac #66344) #14621

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
ArkaPrabhaChowdhury wants to merge 1 commit into
base: trunk
Choose a base branch
from
+127 −10
Open

Icons: Prevent unregistering core-admin icons (Trac #66344) #14621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions src/wp-includes/class-wp-icon-collections-registry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ public function register( $collection_slug, $collection_properties ) {
* Unregisters an icon collection.
*
* Any icons registered under the given collection are also unregistered.
* The core-admin collection cannot be unregistered.
*
* @since 7.1.0
*
Expand All @@ -174,6 +175,15 @@ public function unregister( $collection_slug ) {
return false;
}

if ( 'core-admin' === $collection_slug ) {
_doing_it_wrong(
__METHOD__,
__( 'The "core-admin" icon collection cannot be unregistered.' ),
'7.2.0'
);
return false;
}

$icons_registry = WP_Icons_Registry::get_instance();
foreach ( $icons_registry->get_registered_icons() as $icon ) {
if ( isset( $icon['collection'] ) && $icon['collection'] === $collection_slug ) {
Expand Down
11 changes: 11 additions & 0 deletions src/wp-includes/class-wp-icons-registry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ public function register( $icon_name, $icon_properties ) {
/**
* Unregisters an icon.
*
* Icons in the core-admin collection cannot be unregistered.
*
* @since 7.1.0
*
* @param string $icon_name Namespaced icon name in the form "collection/icon-name"
Expand All @@ -248,6 +250,15 @@ public function unregister( $icon_name ) {
return false;
}

if ( 'core-admin' === $this->registered_icons[ $icon_name ]['collection'] ) {
_doing_it_wrong(
__METHOD__,
__( 'Icons in the "core-admin" collection cannot be unregistered.' ),
'7.2.0'
);
return false;
}

unset( $this->registered_icons[ $icon_name ] );
return true;
}
Expand Down
84 changes: 84 additions & 0 deletions tests/phpunit/tests/icons/wpIconsRegistry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,90 @@ public function test_unregister_unknown_icon() {
$this->assertFalse( $this->registry->unregister( 'test-collection/ghost' ) );
}

/**
* @ticket 66344
*
* @covers ::unregister
*
* @expectedIncorrectUsage WP_Icons_Registry::unregister
*/
public function test_cannot_unregister_core_admin_icon() {
$this->registry->register(
'core-admin/test-icon',
array(
'label' => 'Admin icon',
'content' => '<svg><path d="M0 0"/></svg>',
)
);
$before = wp_get_icon( 'core-admin/test-icon' );

$this->assertNotEmpty( $before );
$this->assertFalse( wp_unregister_icon( 'core-admin/test-icon' ) );
$this->assertSame( $before, wp_get_icon( 'core-admin/test-icon' ) );
}

/**
* @ticket 66344
*
* @covers WP_Icon_Collections_Registry::unregister
*
* @expectedIncorrectUsage WP_Icon_Collections_Registry::unregister
*/
public function test_cannot_unregister_core_admin_collection() {
$this->registry->register(
'core-admin/test-icon',
array(
'label' => 'Admin icon',
'content' => '<svg><path d="M0 0"/></svg>',
)
);
$collections = WP_Icon_Collections_Registry::get_instance();
$collection = $collections->get_registered( 'core-admin' );
$before = wp_get_icon( 'core-admin/test-icon' );

$this->assertNotEmpty( $before );
$this->assertFalse( wp_unregister_icon_collection( 'core-admin' ) );
$this->assertSame( $collection, $collections->get_registered( 'core-admin' ) );
$this->assertSame( $before, wp_get_icon( 'core-admin/test-icon' ) );
}

/**
* @ticket 66344
*
* @covers ::unregister
* @covers WP_Icon_Collections_Registry::unregister
*/
public function test_can_unregister_core_icons_and_collection() {
$collections = WP_Icon_Collections_Registry::get_instance();
$collection = $collections->get_registered( 'core' );
unset( $collection['slug'] );

$this->registry->register(
'core/test-icon',
array(
'label' => 'Core icon',
'content' => '<svg></svg>',
)
);
$this->registry->register(
'core/cascade-icon',
array(
'label' => 'Core icon',
'content' => '<svg></svg>',
)
);

try {
$this->assertTrue( wp_unregister_icon( 'core/test-icon' ) );
$this->assertFalse( $this->registry->is_registered( 'core/test-icon' ) );
$this->assertTrue( wp_unregister_icon_collection( 'core' ) );
$this->assertFalse( $collections->is_registered( 'core' ) );
$this->assertFalse( $this->registry->is_registered( 'core/cascade-icon' ) );
} finally {
$collections->register( 'core', $collection );
}
}

/**
* @ticket 64651
*
Expand Down
32 changes: 22 additions & 10 deletions tests/phpunit/tests/icons/wpRestIconsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ class Tests_REST_WpRestIconsController extends WP_Test_REST_Controller_Testcase
protected static $contributor_id;
protected static $subscriber_id;

/**
* Registry instances restored after each test.
*
* @var array
*/
private $original_registries = array();

public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) {
self::$admin_id = $factory->user->create( array( 'role' => 'administrator' ) );
self::$editor_id = $factory->user->create( array( 'role' => 'editor' ) );
Expand All @@ -34,17 +41,14 @@ public static function wpTearDownAfterClass() {
public function set_up() {
parent::set_up();

/*
* Other suites reset the `WP_Icons_Registry` singleton, wiping the collections and
* icons that `init` only registers once. Replay the registration so order-dependent
* tests pass. `_wp_register_default_icon_collections()` registers every default
* collection at once, so drop whatever survived rather than topping up.
*/
$collections_registry = WP_Icon_Collections_Registry::get_instance();
foreach ( array( 'core', 'core-admin' ) as $collection_slug ) {
if ( $collections_registry->is_registered( $collection_slug ) ) {
$collections_registry->unregister( $collection_slug );
// Use fresh registries, since the core-admin collection cannot be unregistered.
foreach ( array( WP_Icon_Collections_Registry::class, WP_Icons_Registry::class ) as $class ) {
$this->original_registries[ $class ] = $class::get_instance();
$instance_property = new ReflectionProperty( $class, 'instance' );
if ( PHP_VERSION_ID < 80100 ) {
$instance_property->setAccessible( true );
}
$instance_property->setValue( null, null );
}
_wp_register_default_icon_collections();
_wp_register_default_icons();
Expand All @@ -56,6 +60,14 @@ public function tear_down() {
wp_unregister_icon_collection( $slug );
}
}
foreach ( $this->original_registries as $class => $instance ) {
$instance_property = new ReflectionProperty( $class, 'instance' );
if ( PHP_VERSION_ID < 80100 ) {
$instance_property->setAccessible( true );
}
$instance_property->setValue( null, $instance );
}
$this->original_registries = array();
parent::tear_down();
}

Expand Down
Loading