when we print in page.tpl.php "render($page['content'])" in the table
script in "tableselect.js" doing multiple execution
in archive theme "tableselect_bug_theme" for demonstrating this bug
1) install theme
2) and go to "admin/content" or "admin/people" and you can see this bug
OR use this code:
$header = array(
array('class' => array('select-all')),
'1',
);
$rows = array(
array('<input type="checkbox" />', 2),
array('<input type="checkbox" />', 3),
);
print '<table><tr><td>'.theme('table', array('header' => $header, 'rows' => $rows)).'</td></tr></table>';
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | tableselect-1319420-13.patch | 502 bytes | nod_ |
| #13 | tableselect-1319420-13-D7.patch | 482 bytes | nod_ |
| #6 | table.png | 25.01 KB | droplet |
| #5 | tableselect-1319420-5.patch | 440 bytes | nod_ |
| #5 | tableselect-1319420-5-D7.patch | 420 bytes | nod_ |
Comments
Comment #0.0
maxrys commentedcorrect text
Comment #1
maxrys commented+code
Comment #2
nod_issue in
Drupal.behaviors.tableSelectselector. Might want to selectth.select-alland climb up the tree to the firsttableelement. The following seems to work well.$('th.select-all', context).closest('table').once('table-select', Drupal.tableSelect);Ends up being faster than using
:has()in the selector.The obvious question is why you'd want to have nested tables, but I guess it's still a proper bug.
Comment #3
oriol_e9gGo with a patch to test it
Comment #4
droplet commentedbasically 2 problem
ONE: above report
TWO: .select-all checkbox would checked all page's tables
suggest in #2 solve half of the problem. but still need to patch to get .sticky-header works (check table's .select-all, .sticky-header should also checked automatically.)
Comment #5
nod_That should not be the case, all selectors are scoped with the table element, now that it's the right table there shouldn't be any issue.
Do you have a example for #2 ?
Comment #6
droplet commentedlatest problem is .sticky-header needs to checked.
Comment #7
maxrys commentedComment #8
aspilicious commentedNeeds to be fixed in D8 first
Comment #9
nod_What's pointed out in #4 is the original Drupal behavior, not a bug introduced by a change of selectors. #4 is an unrelated feature request.
Please test #5 and confirm.
Comment #10
nod_tagging
Comment #11
droplet commentedTested again. no problem as I mentioned. Sorry
Comment #12
sunI did not understand why this selector plus traversing is needed by merely looking at the patch.
We should add a short inline comment right above the line that explains why this is done this way. Primarily to prevent someone changing it back in the future.
Comment #13
nod_I'm just reversing the traversal,
:hasis a proprietary jquery selector that can't be optimized by querySelectorAll (on top of being buggy in this case), so the code went from basically:$('table', context).filter(function () { return $(this).find('th.select-all').length !== 0; });Which select the all tables having a
th.select-allchild somewhere, to:$('th.select-all', context).closest('table');Which select the innermost table having a
th.select-alland it's much faster, an element having only one immediate parent possible.That's what it should have looked like from the beginning. I'd be more interested in a comment explaining why it's been done this way instead of the other. I'm not sure why would someone revert working code to broken code but here is the comment.
Comment #14
nod_Just added a comment and cleared that up, reverting to rtbc.
Comment #15
dries commentedThe extra comment should have addressed sun's concerns. Committed to 8.x. Moving to 7.x for Angie to consider.
Comment #16
webchickLooks fairly harmless, so committed and pushed to 7.x. Thanks!
Comment #17.0
(not verified) commented+ code