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.

Files: 
CommentFileSizeAuthor
#8 drupal-642734-8.patch3.91 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 39,153 pass(es).
[ View ]
#4 core-js-simpletest-table-642734-4.patch3.93 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 34,218 pass(es).
[ View ]
simpletestBehaviorFix.patch5.06 KByhahn
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch simpletestBehaviorFix.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Title:Ensure JS behaviors attach only onceSimpletest behaviors should process the elements only once
Project:SimpleTest» Drupal core
Version:6.x-2.x-dev» 7.x-dev
Component:Code» simpletest.module
Status:Needs review» Patch (to be ported)

Indeed, but this needs to be fixed in core first, then backported.

Version:7.x-dev» 8.x-dev

Status:Patch (to be ported)» Needs work

We need a patch cleanup (coding standards, docs, proper comments...)

Status:Needs work» Needs review
StatusFileSize
new3.93 KB
PASSED: [[SimpleTest]]: [MySQL] 34,218 pass(es).
[ View ]

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.

Issue tags:+JavaScript clean-up

.

Status:Needs review» Reviewed & tested by the community
Issue tags:+needs backport to D7

Coolio, 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.

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Committed/pushed to 8.x, moving to 7.x for backport.

@nod_ I'm surprised you're surprised ;)

Status:Patch (to be ported)» Needs review
StatusFileSize
new3.91 KB
PASSED: [[SimpleTest]]: [MySQL] 39,153 pass(es).
[ View ]

Rerolled.

#8: drupal-642734-8.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

Yeah! Same as D8.

Status:Reviewed & tested by the community» Fixed

Committed 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).

Also, I noticed that as a result of the patch this line of JavaScript now appears twice:

      var direction = settings.simpleTest[this.id].imageDirection;

Seems harmless, but probably a mistake, so perhaps it's worth a quick followup (Drupal 8 also).

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.

Ah, yes, thanks - I missed that the setting was being modified again 20 lines later in the function :)

Status:Fixed» Closed (fixed)
Issue tags:-needs backport to D7, -JavaScript clean-up

Automatically closed -- issue fixed for 2 weeks with no activity.