Opened 16 months ago
Last modified 30 hours ago
#60495 new enhancement
Following "plugins_list": Add a filter in get_views() in class-wp-plugins-list-table
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 6.9 | Priority: | normal |
Severity: | normal | Version: | 6.3 |
Component: | Plugins | Keywords: | has-patch changes-requested |
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)
Change History (21)
#1
@
16 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
This ticket was mentioned in Slack in #core by oglekler. View the logs.
13 months ago
#4
@
13 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.
13 months ago
#5
- Keywords has-patch added; needs-patch removed
Trac ticket: #60495
@
13 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
@
13 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
@
13 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.
10 months ago
#9
@
9 months 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
.
#10
@
9 months 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.
#11
@
9 months 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.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
4 months ago
#13
@
4 months ago
- Keywords needs-refresh added
As per today's 6.8 bug scrub:
This one needs a small patch refresh to:
- fix some WPCS issues
- update the
@since
tag
And of course some user testing would be more than welcome.
This ticket was mentioned in PR #8294 on WordPress/wordpress-develop by @sukhendu2002.
4 months ago
#14
- Keywords needs-refresh removed
Trac ticket: https://core.trac.wordpress.org/ticket/60495
Patch refresh for: https://github.com/WordPress/wordpress-develop/pull/6637
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
4 months ago
#16
@
4 months ago
- Milestone changed from 6.8 to 6.9
As per today's bug scrub: It appears this ticket still needs user testing. As 6.8 beta 1 is very close, I'm moving it to 6.9. Feel free to move it back to 6.8 if it can be committed by Monday.
#17
@
11 days ago
@soyebsalar01 I removed your comment as AI generated comments that only rephrases the issue doesn't help a bit and are not welcome here since they only add noise.
#19
@
30 hours ago
- Keywords changes-requested added; needs-testing removed
Test Report
Description
❌ This report can't validate that the indicated patch works as expected.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/8294.diff
Environment
- WordPress: 6.9-alpha-60093-src
- PHP: 8.2.28
- Server: nginx/1.27.5
- Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
- Browser: Chrome 137.0.0.0
- OS: Windows 10/11
- Theme: Twenty Twenty 2.9
- MU Plugins: None activated
- Plugins:
- Plugins Filter List 1.0
- Test Reports 1.2.0
Testing Instructions
- First add the code provided in OP, I added as a new plugin
- Add hello dolly to
hello-dolly/hello.php
- Go to Plugins
- 🐞 Problem happens as described in OP
Actual Results with the test
- ❌ Issue is not resolved with patch.
Additional Notes
- Tests seem to be covering a use cases of the new filter (adding a new custom status), but not covering the problem with the repeated statuses that seem to conflict with existing statuses and generate a wrong new status with count 1.
cc @sukhendu2002
Indeed, this would be a good follow-up to [56068]. Moving for 6.6 consideration.
Would you like to suggest a patch for this?