Make WordPress Core

Opened 8 months ago

Last modified 7 days ago

#60495 new enhancement

Following "plugins_list": Add a filter in get_views() in class-wp-plugins-list-table

Reported by: juliobox's profile juliobox Owned by:
Milestone: 6.8 Priority: normal
Severity: normal Version: 6.3
Component: Plugins Keywords: has-patch needs-testing dev-feedback
Focuses: Cc:

Description

Since WP 6.3 there is a new filter named "plugins_list" added in #57278

Using this filter, we can now add a new array key like "my_plugins" and set some plugins in here.
"my_plugins" is now considered as a "status" by the WP behavior, like "all", "recently_activated", etc, the whole list is here : https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-wp-plugins-list-table.php#L49

We can also delete (unset()) one of them if we want too, (like hide the must-use plugins or the "upgraded" tab)

So now WordPress will go through each iteration of the array keys (aka statuses) and create a tab link to get a new view, here : https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-wp-plugins-list-table.php#L495

What's the issue here.
WP will try to display a "text" for each iteration, if there is no case in the switch, it will still display the $text var, and indeed the last used value, aka incorrect value.

But remember line 49, WP sets a list of allowed statuses, this shouldn't be there anymore since the new filter plugins_list allow us to add ANY status using an array key. We have to remove line 49 and modify line 52 to remove the in_array() stuff.

Still need a check to keep the same behavior when a wrong status is loaded? it's already done line 315: https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-wp-plugins-list-table.php#L315

Now to get the correct translated label in get_views() we need a hook line 586 like:

<?php
// ...
default:
    $text = apply_filters( 'plugins_list_status_text', '', $count, $status );
    if ( empty( $text ) ) {
        $text = $status;
    }
    $text .= ' <span class="count">(%s)</span>';
    break;
}

Now please test in WP 6.3.x, using this:

<?php
add_filter( 'plugins_list', 'my_plugins_tab' );
function my_plugins_tab( $plugins ) {
        $plugins['my_plugins'] = $plugins['all']['hello-dolly/hello.php']
        return $plugins;
}

And go to the /plugins.php page. You'll see the last tab with "(1)" and its label is a duplicated one, the same as the last one.
Then click on it: your view is the 'All' one, not the desired one.

Now apply my modifications/patch using this:

<?php
add_filter( 'plugins_list', 'my_plugins_tab' );
function my_plugins_tab( $plugins ) {
        $plugins['my_plugins'] = $plugins['all']['hello-dolly/hello.php']
        return $plugins;
}

add_filter( 'plugins_list_status_text', 'my_plugins_tab_label', 10, 3 );
function my_plugins_tab_label( $text, $count, $type ) {
    if ( 'my_plugins' === $type ) {
        $text = _nx( 'My Plugin', 'My Plugins', $count, 'domain' );
    }
    return $text;
}

Tadaaa, the label is fine and since we remove the allowed status line code, the view is correct.

This ticket now fully finish the job started in #57278!
Thanks for your time and tests and opinions.

Attachments (2)

60495.patch (1.3 KB) - added by sajjad67 4 months ago.
Removed plugin_status in_array() checking and added a default switch case for the plugin tab label and also introduces a new filter.
60495.2.patch (1.3 KB) - added by sajjad67 3 weeks ago.
Sanitizing the user input for plugin status filter

Download all attachments as: .zip

Change History (13)

#1 @juliobox
8 months ago

  • Summary changed from Add a filter for $allowed_statuses and get_views() in class-wp-plugins-list-table to Following "plugins_list": Add a filter in get_views() in class-wp-plugins-list-table

#2 @audrasjb
8 months ago

  • Milestone changed from Awaiting Review to 6.6

Indeed, this would be a good follow-up to [56068]. Moving for 6.6 consideration.
Would you like to suggest a patch for this?

This ticket was mentioned in Slack in #core by oglekler. View the logs.


5 months ago

#4 @oglekler
5 months ago

  • Keywords needs-testing removed

This ticket was discussed during a bus scrub, @sajjad67 volunteered to work on the patch.

Add props to @audrasjb and @hellofromTonya

This ticket was mentioned in PR #6637 on WordPress/wordpress-develop by @khokansardar.


4 months ago
#5

  • Keywords has-patch added; needs-patch removed

Trac ticket: #60495

@sajjad67
4 months ago

Removed plugin_status in_array() checking and added a default switch case for the plugin tab label and also introduces a new filter.

#6 @sajjad67
4 months ago

  • Keywords needs-testing added

@audrasjb and @hellofromTonya Hi, Please look into the patch submitted and testers are very much welcome to test it and provide feedback. I think it might need to work on the filter commenting.

#7 @oglekler
4 months ago

  • Milestone changed from 6.6 to 6.7

@khokansardar and @sajjad67, thank you for the patches! We have 2 days before Beta 1 and no time for testing, so, I have to move this ticket to the next milestone. Let's try to finish the work there.

This ticket was mentioned in Slack in #core-test by ankit-k-gupta. View the logs.


5 weeks ago

#9 @davidbaumwald
3 weeks ago

  • Keywords dev-feedback added

Looking at the latest patch, I think if the $allowed_statuses whitelist check goes away, we'll need to sanitize $_REQUEST['plugin_status'] before using it. Same would need to be done in the new default case before passing to WP_List_Table->get_views_links.

@sajjad67
3 weeks ago

Sanitizing the user input for plugin status filter

#10 @sajjad67
3 weeks ago

@davidbaumwald I believe the only place we are assigning value from user input is in line #50, so adding sanitization to that, other place you mentioned in default case would get the sanitized $status, please correct me if I am wrong.

Last edited 3 weeks ago by sajjad67 (previous) (diff)

#11 @davidbaumwald
7 days ago

  • Milestone changed from 6.7 to 6.8

With 6.7 Beta 1 releasing in a few hours, this is being moved to 6.8 given its recent momentum towards a resolution.

If any committer feels the remaining work can be resolved in time for Beta 1 wishes to assume ownership, feel free to update the milestone accordingly.

Note: See TracTickets for help on using tickets.