Make WordPress Core

Opened 7 years ago

Last modified 8 weeks ago

#35833 new defect (bug)

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

Reported by: braad's profile Braad Owned by:
Milestone: 6.3 Priority: normal
Severity: normal Version: 4.4.2
Component: Quick/Bulk Edit Keywords: has-screenshots has-patch needs-testing
Focuses: ui, javascript, administration Cc:

Description

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.

Attachments (6)

wp-quick-edit-issue.png (138.1 KB) - added by Braad 7 years ago.
Screenshot of the issue, after toggling open the quick edit
inline-edit-post.diff (2.7 KB) - added by Braad 7 years ago.
First pass at using more specific selectors
wp-tax-quick-edit-issue.png (235.9 KB) - added by Braad 7 years ago.
Screenshot of the issue on taxonomy edit screens, after toggling open the quick edit
test-35833.php (402 bytes) - added by Braad 7 years ago.
A quick plugin for testing this issue, simply adds a table.widefat in the Help tabs
35833.diff (4.3 KB) - added by Braad 7 years ago.
Improved patch that uses table.wp-list-table in place of table.widefat for both post and taxonomy edit screens
35833.2.diff (6.1 KB) - added by oglekler 3 months ago.

Download all attachments as: .zip

Change History (25)

@Braad
7 years ago

Screenshot of the issue, after toggling open the quick edit

#1 @Braad
7 years ago

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.

#2 @Braad
7 years ago

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
7 years ago

First pass at using more specific selectors

#3 @Braad
7 years ago

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.

#4 @Braad
7 years ago

  • Keywords has-screenshots has-patch added

#5 @Braad
7 years ago

  • Summary changed from Including a table with the "widefat" class in Help tabs causes the quick edit UI to fail to 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
7 years ago

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

@Braad
7 years ago

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

@Braad
7 years ago

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

#6 @Braad
7 years ago

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 :)

Last edited 7 years ago by Braad (previous) (diff)

#7 @Braad
7 years ago

  • Focuses ui javascript administration added
  • Keywords dev-feedback added

#8 @oglekler
4 months ago

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.

#9 @webcommsat
4 months ago

  • Keywords needs-testing added

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.

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


4 months ago

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


4 months ago

#12 @SergeyBiryukov
4 months ago

  • Milestone set to 6.3

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


3 months ago

@oglekler
3 months ago

#14 @oglekler
3 months ago

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

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


3 months ago
#15

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

#16 @oglekler
2 months ago

  • Keywords dev-feedback removed

#17 @oksankaa
8 weeks ago

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
Last edited 8 weeks ago by oksankaa (previous) (diff)

#18 @oglekler
8 weeks ago

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

#19 @oksankaa
8 weeks ago

@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.

Note: See TracTickets for help on using tickets.