Speed Up Simpletest.js

quicksketch - November 17, 2008 - 04:12
Project:Drupal
Version:7.x-dev
Component:simpletest.module
Category:task
Priority:normal
Assigned:Unassigned
Status:closed
Description

This patch makes simpletest.js perform its collapsing table rows trick faster. The current implementation causes slight delays on slower machines before it opens/closes, and uses unnecessarily intensive "fadeIn" and "fadeOut" jQuery methods when hiding/showing, even though table rows aren't capable of being faded in many browsers. I think the intention of using them might have been to simply intentionally slow down the effect. This patch takes a different approach and simply toggles visibility and uses a timer to control the expansion of rows, rather than intentionally grinding CPU cycles just to cause a delay.

Chx informs me this problem is especially noticeable in Opera. And in Konqueror the table expansion doesn't work at all. This patch works in Firefox, Opera, Konqueror, IE 6/7.

AttachmentSize
simpletest_faster_js.patch2.61 KB
Testbed results
simpletest_faster_js.patchfailedFailed: Failed to apply patch. Detailed results

#1

quicksketch - November 17, 2008 - 04:52
Title:Speed Up Simpletest.js With neater code» Speed Up Simpletest.js

"neater code" seems a little biased. We'll just say it's less intensive. :)

#2

System Message - November 17, 2008 - 05:30
Status:needs review» needs work

The last submitted patch failed testing.

#3

boombatower - November 17, 2008 - 07:46

Hint...this needs to be created from drupal root.

bot eats patches that are relative :)

#4

quicksketch - November 18, 2008 - 19:21
Status:needs work» needs review

Thanks boombatower. :)

AttachmentSize
simpletest_faster_js.patch 2.67 KB
Testbed results
simpletest_faster_js.patchpassedPassed: 7252 passes, 0 fails, 0 exceptions Detailed results

#5

boombatower - November 19, 2008 - 09:38
Status:needs review» reviewed & tested by the community

Looks good, works fine for me as well.

#6

webchick - November 20, 2008 - 05:16
Status:reviewed & tested by the community» needs work

Since we're taking the opportunity to clean up the code, can we add a smattering of comments to explain what's going on?

#7

quicksketch - November 20, 2008 - 05:26
Status:needs work» reviewed & tested by the community

Same patch with a few more comments. Webchick made a solid argument that a lack of comments in the file doesn't justify a continued absence. So while we're in here add a few lines describing what we're doing.

AttachmentSize
simpletest_faster_js.patch 3.13 KB

#8

quicksketch - November 20, 2008 - 05:30

Typo in the last patch's comments.

AttachmentSize
simpletest_faster_js.patch 3.13 KB

#9

webchick - November 20, 2008 - 05:53
Status:reviewed & tested by the community» fixed

Wow, I can actually understand what's going on in that file now. :O

Has the quicksketch + boombatower seal of approval, chx confirmed it vastly speeds things up in Opera (though I was unable to test since Opera is fubar on my machine).

Made a couple minor adjustments to the comments and committed to HEAD. Thanks. :)

#10

System Message - December 4, 2008 - 05:54
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.