Adding a table with the "widefat" class on post and taxonomy list table screens causes the quick edit UI and bulk edit to fail

[Braad]

This is a very strange issue. On a fresh WP install running twentysixteen with no plugins activated, including this code causes the quick edit UI to miscalculate its width and end up looking wacky:

add_action( 'load-edit.php', 'xxx_test_table_in_help_tab' );
function xxx_test_table_in_help_tab() {
    $test_table_args = array(
        'id'       => 'test_help_table',
        'title'    => 'Something',
        'content'  => '<table class="widefat"><tr><td>Something</td></tr></table>',
    );
    get_current_screen()->add_help_tab( $test_table_args );
}

I'm attaching a screenshot of what the quick edit UI looks like with this code in place. Removing the class "widefat" restores the quick edit UI to normal. As strange as it sounds, I believe the presence of the widefat class is causing something in the quick edit JS to miscalculate. I haven't found the exact line where the issue is, but it seems to all come down to the presence of the "widefat" class.

[Braad]

Screenshot of the issue, after toggling open the quick edit

[Braad]

If anyone wants to test this without running that code, you can even just inspect element in your browser and paste this into one of the Help tabs .help-tab-content divs:

<table class="widefat"><tr><td>Something</td></tr></table>

Then try toggling the quick edit to see the issue. This is happening for me in Chrome.

[Braad]

The issue appears to be originating in /wp-admin/js/inline-edit-post.js, where this selector is used throughout that file:

$('table.widefat')

Since these aren't scoped to just the quick edit section a <table> with the class "widefat" inside the Help tabs (or really anywhere else on the page) is getting targeted also.

There is even a selector that is just ".widefat" on line 291, which could potentially cause other side effects when the class is used on inputs and other elements anywhere on the page.

[Braad]

First pass at using more specific selectors

[Braad]

Just created my first patch that attempts to fix this issue. I'm not sure what the official method is for minifying core JS so I haven't submitted a minified version of the file yet, but trying the updated code in that file works for me.

The changes I made include switching all instances of the generic $( 'table.widefat' ) and $( '.widefat' ) to $( 'table.wp-list-table.widefat' ). I'm hoping this is safe to do, but it's possible that the "widefat" class is no longer needed in the selector at all and we can just do $( 'table.wp-list-table' ).

I don't expect my patch will be the final one but I wanted to get the ball rolling. If anyone can point me in the right direction to minify the file I can submit another patch that includes that, or if there are other issues with my patch I'm happy to make any necessary changes to take this over the finish line.

[Braad]

Screenshot of the issue on taxonomy edit screens, after toggling open the quick edit

[Braad]

A quick plugin for testing this issue, simply adds a table.widefat in the Help tabs

[Braad]

Improved patch that uses table.wp-list-table in place of table.widefat for both post and taxonomy edit screens

[Braad]

After digging a bit deeper I discovered that the table.widefat and .widefat selectors were also being used on taxonomy edit screens in /wp-admin/js/inline-edit-tax.js, and the issue with the quick edit UI was also happening there.

I also discovered that on the posts list table screens the bulk edit was broken (checking box for one or more posts, setting "bulk edit actions" dropdown to "edit" and clicking apply does nothing) in addition to the issue with the quick edit UI happening.

All of these issues happen when there is a second table with the class "widefat" anywhere on the post and taxonomy list table screens, not just in the Help tabs.

I've uploaded a screenshot of the quick edit issue happening on taxonomy edit screens, and I've uploaded test-35833.php as a quick test plugin you can activate to see the issue happening (activate and try to use the quick edit on posts, pages, categories, or tags to see the issue).

I'm also uploading a second patch that fixes a typo in the first patch, switches to consistently using table.wp-list-table as the selector (getting away from selecting by .widefat completely), and updates inline-edit-tax.js also, solving this problem for both post and taxonomy list table screens.

Looking at similar tickets it doesn't seem that people are submitting minified versions of JS files in patches, so I think this new patch has everything it needs. I did grab the develop branch and get the grunt tasks working and all qunit tests still pass.

If there is anything else this patch needs, please let me know. I'm happy to keep working on it. This would be my first core contribution if it gets merged, and I'm ready to do anything necessary to take it over the finish line :)

[oglekler]

Hi @Braad thank you for the ticket and the patch,

I agree that .widefat doesn't look like the right selector for JS DOM manipulations and should be used for styling purposed only. But this change will not bring any improvements other than the ability to use this class for styling purposes elsewhere. I understand that your code is just an example and there was a reason to place a table into the Help tab, but the same wide result can be achieved with inline styles for not to make things too complicated and not to add a new style file into the admin purely for this purpose. On the other hand, if this patch to be applied, someone will need to test inline post editing script for an unexpected regression even if it looks like nothing can be broken.

I suggest leaving jQuery selector as is and return to this case if the inline editing script will be reworked for another reason.

[webcommsat]

Component review today
The patch needs testing. @oglekler has left some notes above.
This is one of the tickets to be tested at WCAsia Contributor Day. This could be an easier ticket to test, and if this works, then to fix. Note from @oglekler: someone will need to test inline post editing script for an unexpected regression even if it appears that nothing is broken.

[oglekler]

Patch (35833.2.diff) is updated with regard to new files location, plus to remove '.widefat' selector from JS files completely. With this selector was another compatability issue in src/js/_enqueues/admin/inline-edit-tax.js, see: https://core.trac.wordpress.org/ticket/35664#comment:13

For testing:
src/js/_enqueues/admin/edit-comments.js - Comments list
src/js/_enqueues/admin/inline-edit-post.js - Posts list
src/js/_enqueues/admin/inline-edit-tax.js - Taxonomies lists
src/js/_enqueues/admin/post.js - Comments list under the Classic editor

PR is based on the previous PR, patch is updated with regard to new files location,
.widefat selector is removed from JS files and replaced by classes which reference DOM elements and not how they should look.

Trac ticket: https://core.trac.wordpress.org/ticket/35833

[oksankaa]

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/4226

Environment

• OS: macOS 12.2.1
• Web Server: Nginx
• PHP: 7.4.33
• WordPress: 6.3-alpha-55505-src
• Browser: Chrome 111.0.5563.146
• Theme: Twenty Twenty-One (but it doesn't matter)
• Active Plugins:
  • Classic Editor 1.6.3
  • Query Monitor 3.12.1
  • Test plugin by Braad

Actual Results

• Issue resolved with the patch.

The changes are applied in these files:

• src/js/_enqueues/admin/edit-comments.js
• src/js/_enqueues/admin/inline-edit-post.js
• src/js/_enqueues/admin/inline-edit-tax.js
• src/js/_enqueues/admin/post.js

Then the npm run build:dev command was run.

Tested the issue is no longer reproduced in these areas:

• ✅ Comments list
• ✅ Taxonomies lists
• ✅ Single post editing and its Comments list in the Classic editor
• ✅ Comments list for posts/pages under the Classic editor
• ✅ Comments list for media items under the Edit Media page

[oglekler]

Hi @oksankaa,
Did you run npm run build:dev?
These changes are in source files and actual scripts need to be built.

[oksankaa]

@oglekler Oh, you are right! I didn't know I needed to run that command, thanks for pointing this out.

I've updated the test report above (to avoid unnecessary duplication), plus added yet another place I checked.

[oglekler]

From my point of view, this is ready to go into 6.3. But the final patch is mine, so, please take a look.

[oglekler]

I've re-checked the patch and marking this for review to commit.

[mukesh27]

This ticket was discussed during the bug scrub.

@SergeyBiryukov could you please take look?

[azaozz]

Replying to oglekler:

Sure, the patch makes sense here and seems to work well. Just wondering if the same problem may happen because some other plugin is adding tables with a wp-list-table class on these screens. Seems possible as there are quite a few matches in a ​WP Directory search. Perhaps a new class name should be added to these list tables specifically to target them from the WP JS.

[oglekler]

@azaozz it may happen if some of these plugins are adding similar content into the same pages, for example 'inline-edit-tax' will be only enqueued on the edit-tags.php page. I think it is unlikely, but I also believe that the right approach to separate classes for CSS and JS and don't mix up them. What is your suggestion, change it now?

[azaozz]

Replying to oglekler:

Yes, you're right. Best would be to change it. Yea, it is "best practice" to use separate classes that are used only in JS.
(In theory that won't stop some plugin from using it for something else, but that can happen even with the best intentions.)

[oglekler]

I had no time to work on this further, so I am moving it into 6.4, and now it looks more like enhancement that bug fix.

[oglekler]

The scope of the ticket is clear now, and there is already a patch with which work can be continued, so, I am marking this as good-first-bug.

[oglekler]

Because I didn't have time to work on this patch and no one picked up this so far, I am moving this ticket into 6.5

[oglekler]

This ticket was discussed during bug scrub.

This is a straightforward patch, it most likely will need rebase due to other js patch we are working on, but the change is simple enough — to add dedicated class to list tables and to replace it in the scripts. JS files that heed changes are there: ​https://github.com/WordPress/wordpress-develop/pull/4226/files

@azaozz Do we have some naming convention for this JS used classes?

I am removing myself from owners for not to confuse new contributors that this ticket "is taken".

Add props to @jorbin

[webcommsat]

Highlighted at today's bug scrub (December 19, 2023). There is some advice above on updating a patch. If you can give time to help move this forward, please comment on the ticket.

@Braad thanks for highlighting and the work you have done to progress this ticket. There are some comments above on the latest patch if you would are able to help move this forward to 6.5.

@azaozz grateful if you could look at the question in the above comment from Olga on naming conventions.

[azaozz]

Replying to oglekler:

Do we have some naming convention for this JS used classes?

Don't think so. As with any other names in WP, it would be quite nice to make them self-documenting as much as possible, and even add an inline comment about the expected use.

This is a straightforward patch...

Right. Still thinking this will be much better if a specific classmane is used instead of .wp-list-table. Perhaps something like inline-edit-list-table or maybe wp-inline-edit?

[swissspidy]

Moving out of the milestone as there is no consensus or traction and 6.5 Beta is approaching.

[oglekler]

This ticket was discussed during a big scrub, @lamarajan express the interest to make a new patch. Let's do this )

[lamarajan]

Hello core team,

I am working on this ticket. I will post update here.

Thanks,
Rajan Lama (@lamarajan )

This PR has fixed the issue for broken table structure for quick edit when custom table has been created in page / list editor with class ".widefat".

Trac ticket: https://core.trac.wordpress.org/ticket/35833

[lamarajan]

This it Patch file to fix the issue related to class widefat.

[lamarajan]

Hello @oglekler,
I have fixed the bug. Actually, it was the late binding issue. So, I have fixed the code, created a .diff file for patch and attached with this ticket.

I have also created a PR. Here is the PR link
​https://github.com/WordPress/wordpress-develop/pull/6599

If you have need more info let me know.

Thanks,
Rajan Lama (@lamarajan)

[oglekler]

Hi @lamarajan, thank you for the patch and, please, forgive me for delay in answering. Let's wait what @azaozz is thinking, but I believe that fixing layout with JS is not the correct way in general — layout structure can change, and it will stop working, no one will be expecting fix in JS. As we discussed previously, I was expecting complete removal of widefat class from JS (there is already a patch as an example) with naming change as it was proposed by in #comment:36

[azaozz]

Replying to oglekler:

I believe that fixing layout with JS is not the correct way in general

Yes, thinking the same. This patch has a similar problem to the previous one as mentioned above: https://core.trac.wordpress.org/ticket/35833#comment:25.

Still thinking it would be better to add a specific HTML class and not reuse any of the existing classes/structure. (Btw this should be pretty easy to do, seems it also needs a very small change in PHP.)

[oglekler]

We have 2 days before Beta 1, and sadly, I have to move this ticket to the next milestone.

[lamarajan]

Hello @oglekler and @azaozz,

Thank you for your opinions, but I think using JS is better solution here. Because if we came on with multi layered nested table. It will get difficult to handle with only CSS.

Thats why, I have written the JS, that is short and sweet. But still, if you like to try my patch, that will make me happy :) .

If you have any questions let me know.

Thank you.

Best Regards,
@lamarajan