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.When using modules that make additional calls to Drupal.attachBehaviors() the simpletest JS behaviors may be attached multiple times leading to unexpected behaviors and/or breakage (e.g. 2 toggles per click which leads to the same state).
This patch adjusts the JS behaviors so that they attach only once against the DOM elements. See http://drupal.org/node/114774#javascript-behaviors. Patched against CVS checkout of DRUPAL-6--2.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | drupal-642734-8.patch | 3.91 KB | tim.plunkett |
| #4 | core-js-simpletest-table-642734-4.patch | 3.93 KB | nod_ |
| simpletestBehaviorFix.patch | 5.06 KB | yhahn | |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedIndeed, but this needs to be fixed in core first, then backported.
Comment #2
TR CreditAttribution: TR commentedComment #3
oriol_e9gWe need a patch cleanup (coding standards, docs, proper comments...)
Comment #4
nod_That's surprising that nobody ran into this in a couple of years. It bitted me working on #1423500: Use RequireJS to load all JS.
added: .once, declared a leaking variable, re-scoped part of the code to make it work.
It's just a working patch, not a pretty one. Ideally it should be rewritten, having a variable animation time is really confusing.
Comment #5
nod_.
Comment #6
sunCoolio, thanks!
Shameless plug: I'm experimenting with some SimpleTest UI improvements in http://drupal.org/project/admin_ux - users + reviews + whatnot + ultimately core patches based on that welcome! ;)
Lastly, this should be backported.
Comment #7
catchCommitted/pushed to 8.x, moving to 7.x for backport.
@nod_ I'm surprised you're surprised ;)
Comment #8
tim.plunkettRerolled.
Comment #9
mgifford#8: drupal-642734-8.patch queued for re-testing.
Comment #10
oriol_e9gYeah! Same as D8.
Comment #11
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/e0a0b94
Not sure if we want to follow up with this in the Drupal 6 SimpleTest module queue also (where this issue started originally).
Comment #12
David_Rothstein CreditAttribution: David_Rothstein commentedAlso, I noticed that as a result of the patch this line of JavaScript now appears twice:
Seems harmless, but probably a mistake, so perhaps it's worth a quick followup (Drupal 8 also).
Comment #13
nod_Actually no it's required, if we remove the first one we can't get the right image. And if we remove the other one the group can't be toggled back up.
More cleanup would be rewritting it from JS settings to better HTML.
Comment #14
David_Rothstein CreditAttribution: David_Rothstein commentedAh, yes, thanks - I missed that the setting was being modified again 20 lines later in the function :)