WordPress.org

Make WordPress Core

#39739 closed defect (bug) (fixed)

Select All Checkbox Not Working

Reported by: reldev Owned by: SergeyBiryukov
Milestone: 4.7.3 Priority: normal
Severity: normal Version: 4.7
Component: Administration Keywords: has-patch commit fixed-major
Focuses: javascript Cc:

Description (last modified by SergeyBiryukov)

I'm seeing a problem that appears to be related to the change introduced by ticket #37973 in 4.7. I did not see this issue on 4.6.2, but have verified it also exists on 4.7.1 and 4.7.2.

In short, I have a "select all" checkbox in a table on an admin page that is not functioning properly if the checkbox is within a nested table. If I click the checkbox, it is never "checked" and the checkboxes below in the table are never checked either. I've simplified my html quite a bit in order to isolate the problem and here is an HTML snippet that can be used to reproduce the problem:

<table class="form-table">
  <tbody>
    <tr>
      <td>
        <table class="wp-list-table fixed widefat striped">
          <thead>
            <tr>
              <th scope="col" class="manage-column check-column">
                <label><input type="checkbox" class="check-all"></label>
              </th>
              <th scope="col">
                Name
              </th>
            </tr>
          </thead>
          <tbody id="the-list">
            <tr>
              <th scope="row" class=" check-column">
                <input type="checkbox" id="cb-select-1" name="nslb_import_rows" class="nslb-input" value="1">
              </th>
              <td>
                Steven<input type="hidden" name="s_1_1" class="nslb-input" value="Steven">
              </td>
            </tr>
            <tr>
              <th scope="row" class=" check-column">
                <input type="checkbox" id="cb-select-2" name="nslb_import_rows" class="nslb-input" value="2">
              </th>
              <td>
                David<input type="hidden" name="s_2_1" class="nslb-input" value="David">
              </td>
            </tr>
          </tbody>
        </table>
      </td>
    </tr>
  </tbody>
</table>

What's happening specifically is that the code in common.js on line 421 at the 4.7+ level ($body.on('click'...) is firing when I click the "manage-column check-column" checkbox because it's in a <tbody> element (nested as in my example above). The code on line 430 in common.js at the 4.6.2 level that 37972 replaced does not fire when I select the same checkbox and it functions properly. The code on line 443 and following in common.js at the 4.7+ level is attempting to determine how to toggle any "select all" checkboxes in a table header or footer but since we're incorrectly in this routine, "false" is being returned for the most part meaning the "manage-column check-column" checkbox will never be checked and the other checkboxes in the following <tbody> will not be checked either. It seems the code at the 4.7+ level is assuming checkboxes within a <tbody> would not also be a "select all" checkbox, but bounded by a <thead> and <tfoot> where the select all boxes would typically be. Because I've nested a table within another <tbody>, this code incorrectly fires.

Attachments (1)

39739.diff (571 bytes) - added by swissspidy 17 months ago.

Download all attachments as: .zip

Change History (14)

#1 @reldev
17 months ago

Sorry, I inadvertently referenced the ticket that introduced the bug as 37972 and it should 37973. The link to the ticket is correct however.

Last edited 17 months ago by reldev (previous) (diff)

#2 @afercia
17 months ago

Changing $body.on( 'click', 'tbody .check-column :checkbox', function( event ) { in $body.on( 'click', 'tbody > .check-column :checkbox', function( event ) { should fix it. @reldev could you please check if that addresses your specific use case? Even if I'm not sure nested tables are a good thing, if the fix is so simple I'd be in favor of fixing it ;)

#3 @SergeyBiryukov
17 months ago

  • Component changed from General to Administration
  • Description modified (diff)

#4 @SergeyBiryukov
17 months ago

  • Milestone changed from Awaiting Review to 4.7.3

#5 @reldev
17 months ago

@afercia I can verify the fix you provided takes care of this problem. Thanks so much for the quick response!

@swissspidy
17 months ago

#6 @swissspidy
17 months ago

  • Keywords has-patch added

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


16 months ago

#8 @dd32
16 months ago

  • Keywords commit added

Although nested tables like this is an edge case, I see no reason not to make the selector more specific to ensure it works. 39739.diff looks good to me.

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


16 months ago

#10 @jnylen0
16 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to assigned

Handing this off to @SergeyBiryukov for commit per above Slack discussion.

#11 @SergeyBiryukov
16 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 40118:

Plugins: After [38703], adjust the selector for checkbox selection to account for nested tables.

Props afercia, swissspidy, reldev.
Fixes #39739.

#12 @SergeyBiryukov
16 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

#13 @SergeyBiryukov
16 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 40119:

Plugins: After [38703], adjust the selector for checkbox selection to account for nested tables.

Props afercia, swissspidy, reldev.
Merges [40118] to the 4.7 branch.
Fixes #39739.

Note: See TracTickets for help on using tickets.