Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The version of the tabs plugin we ship with is aging. It would be good to be able to take advantage of several improvements that are in the current release.
Should we make Tabs dependent on jQuery Update, enabling us to use the current version of the tabs plugin (and, maybe, to remove the plugin from our CVS and instead point to separate downloads)?
Comment | File | Size | Author |
---|---|---|---|
#14 | jstools-DRUPAL-5.jquery.patch | 1.43 KB | sun |
#7 | jstools_jsquery.js_.patch | 565 bytes | WorldFallz |
Comments
Comment #1
cpelham CreditAttribution: cpelham commentedSounds good to me. It's easy enough to install jQuery Update module. Does anyone know if jQuery Update causes problems for other modules?
Comment #2
Wim LeersComment #3
liquidcms CreditAttribution: liquidcms commentedbut jquery_update is also very old.. we should work with jq 1.2, jq_update is only 1.1
Comment #4
nedjoThe 6.x version of Tabs module is now available. No need to rework the 5.x version I think.
Comment #5
WorldFallz CreditAttribution: WorldFallz commentedAny chance you will revisit this decision? With the slower than normal migration to D6, D5 is going to be around for a while and jquery_update is being reworked (DRUPAL-5--2 branch) to include jquery 1.2.3-- which breaks tabs and collapsiblock from jstools. I'm sure the issues have been solved already for the d6 version, and I took a look at the code, but I couldn't figure out what needed to be backported.
Comment #6
Wim LeersI'm pretty sure that won't happen. It *seems* nobody is moving to D6, but in fact everybody is preparing to make the jump.
I'll leave this open for nedjo to confirm, but your chances are *very* slim ;)
Comment #7
WorldFallz CreditAttribution: WorldFallz commentedSeems Katbailey has found both the problem and the fix (see http://drupal.org/node/156221#comment-854274). I've done some limited testing on a fresh d5.7 with the recommended jstools 1.1 release and it doesn't seem to break anything. I've attached a patch for the fix in case you're willing to consider a patch ;-)
Comment #8
bbenone CreditAttribution: bbenone commentedThanks, WorldFallz, for the patch! So far it seems to be working for us as well.
I vote for it being put into the current 5.x release! I tend to agree that a large number of people will be in 5.x for a while; and thus it may be a little early to stop supporting 5.x modules. After all, we are not really asking for a new feature here, just a bug fix :)
Comment #9
nedjoThanks, I'll apply this the next time I'm applying changes.
Comment #10
katbailey CreditAttribution: katbailey commentedHold on - there's a problem.
This now interferes with collapse.js because the collapse.js that comes with jstools and adds the "collapse-processed" class to the fieldset legends is causing the fieldset-wrapper div to be created twice, with one nested inside the other. This breaks the collapse functionality. Here's my theory on why this is only a problem when the above patch is applied.
collapse.js in jstools replicates the function in misc/collapse.js which adds the clickable legend and the fieldset-wrapper div. However, this script is called after the original version of the js, which will already have added these elements, and so the check to make sure they're not already processed before applying the behavior is ineffectual. I think that with the unpatched code in jstools.js (line 233), the lines below never actually get called, because of the problem with the context being incorrectly passed in as the entire jquery object (see here), whereas with the code in the patch above, they do get called, on top of the original collapse.js function, which ran before the "collapse-processed" class was added by the jstools version.
Of course, this is based on my incomplete understanding of why this extra collapse.js file is included in jstools and how it's supposed to work with misc/collapse.js but it seems to me that if it's true that it never does anything without the patch and only causes trouble with the patch (which is a genuine fix), then maybe it could be left out altogether?
Comment #11
sunSubscribing
Comment #12
katbailey CreditAttribution: katbailey commentedHmmm, it seems my understanding was limited indeed. My theory was completely wrong :-P
The change in behaviour of collapse.js has nothing to do with the above patch, the problem was being caused by the patch to collapse.js submitted to jQuery Update module.
Sorry for the confusion - the patch in #7 is ok.
Comment #13
katbailey CreditAttribution: katbailey commentedchanging the status back...
Comment #14
sunAttached patch fixes jstools with jquery_update.
I don't know what collapse.js is for specifically, but it duplicates Drupal core's modifications to fieldsets, which is totally error-prone. Disabling it resolves all errors regarding jQuery Update. See #156221: Upgrade to JQuery 1.2.6 for further information.
I could imagine that a /real/ solution for this bug would be to execute jstools' fieldset collapse behaviour *not* on document.ready(), but only if one of jstools' modules require it. Move it out of Drupal.behaviours and convert it into a regular function, or introduce a property for behaviours that indicates whether a behaviour should run on document.ready(). Either way, the current implementation is causing bugs, because both /misc/collapse.js *and* modules/jstools/collapse.js are executed on document.ready(). This results in duplicate legends, duplicate fieldset-wrappers, aso.
Comment #15
jpsalter CreditAttribution: jpsalter commentedsubscribing
Comment #16
sun#261409: tabbed CCK field group are hidden in forms has been marked as duplicate of this issue.
Comment #17
nedjoThanks, applied.
Comment #18
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.
Comment #19
marcus7777 CreditAttribution: marcus7777 commentedThanks for the patch it just what I needed
all the best
Marcus