| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | javascript |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
Drupal.behaviors.tableHeader doesn't respect the context.
To reproduce the bug:
Do an ahah/ajax update on the page, which does NOT replace the table that tableHeaders is attached to.
Call Drupal.attachBehaviors(content) on the new part of the page.
Floating table headers no longer work. If the headers were already floating, they stay floating when they should go away. If they are not floating, they won't start.
The problem is so much code gets run outside of the area protected by
$('table.sticky-enabled thead:not(.tableHeader-processed)', context)Drupal.tableHeaderOnScroll gets redeclared, with blank headers in the closure. The connection to the old headers is lost.
It looks to me like tableheaders.js was written before Drupal.attachBehaviors was added to Drupal 6, and then tweaked just enough to make it work when the entire table is replaced. I suspect it needs to be redesigned to more fully work with the Drupal.attachBehaviors concept.
Comments
#1
A workaround is to add this at the top of Drupal.behaviors.tableHeader
// If there are no sticky tables in this context, just return.if (!$('table.sticky-enabled', context).size()) {
return;
}
#2
I just found out this same issue and still happening on 6.16
#3
Well, for me it was always a bit ambitious to patch drupal core, or even have a production site with a core modified, but here is a humble patch for tableheader.js trying to sort this problem. Although the workaround above does seem to help, I rather tried to do something more general. I believe the problem was that the headers = [] was initialized on each drupal.attachBehvaiors(), thus, loosing the the tables to show on scrolling. I have moved that array to a global one inside the drupal object. Not sure if this has any other implication, but this all seem to on my tests.
One thing that I am concern is what should happen if a table is removed by an AHAH/AJAX call and then Drupal.attachBehaviors(). I think that particular use case is not handled at all by the current tableHeader.js. In anyway, I think my patch does fix the issue reported here, so it's here for review/test.
#4
The last submitted patch, 234377_tableHeader_multiple_attachments.patch, failed testing.
#5
Anybody knows why the patch above was not able to be applie?
#6
hanoii, I've applied changes from your patch by hands (because I've also added support for IE 6 to sticky table headers) and it resolves my problem. Thank You!
About testing, please try to checkout latest drupal 6 code from repository, make patch for it and change version to 6.x-dev.
#7
Trying agains against cvs
#8
Any idea why the test is not being run on the last patch (#7)?
#9
The last submitted patch, 234377_tableHeader_multiple_attachments.patch, failed testing.
#10
I really don't get why this patch is failing to be applied, again, can somebody help me figure out why? Anyway, trying it one more time
#11
The last submitted patch, 234377_tableHeader_multiple_attachments.patch, failed testing.
#12
#10: 234377_tableHeader_multiple_attachments.patch queued for re-testing.
#13
The last submitted patch, 234377_tableHeader_multiple_attachments.patch, failed testing.
#14
Here is simple patch, but it works.
#15
sorry, patch again :)
#16
#17
The last submitted patch, tableheader.patch, failed testing.
#18
#15: tableheader.patch queued for re-testing.
#19
The last submitted patch, tableheader.patch, failed testing.
#20
Your patch looks a lot simpler and better than mine :)
+++ ./tableheader.js 2011-03-25 17:32:41.133114704 +0600@@ -13,7 +13,7 @@ Drupal.behaviors.tableHeader = function
+ var headers = $('table.sticky-header').not($('table', context));
Why are you removing all the tables in the context? What would happen if sticky headers are kept within the context? Is that even possible?
Powered by Dreditor.
#21
subscribing
#22
Hmm... I don't understand why still someone not used solution from #1 . It works perfect.
Attaching the patch, maybe this will hasten elimination of this bug.
#23
#24
Sorry, @Leksat - you can't RTBC your own patch.
Sadly, I have to ask if you've tested to see if this bug exists in 7.x/8.x. If it does, this will need to be fixed in 8.x first...
#25
It's not my patch actually. I stole code from #1 and made a patch from it.
#26
Also, I have tested this patch. It works nice. Why shouldn't we commit it and close the issue?
#27
yeah, let's commit this, it does work.
#28
subscribe...
#29
Does this apply to Drupal 7/8? Was this fixed in Drupal 7/8 already?