Closed (fixed)
Project:
Chaos Tool Suite (ctools)
Version:
7.x-1.x-dev
Component:
Code
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
20 Dec 2011 at 10:38 UTC
Updated:
31 Mar 2012 at 18:30 UTC
Jump to comment: Most recent file
Comments
Comment #1
merlinofchaos commentedOh wow. Yes, yes they absolutely should be included as a library. I must have never updated this code after ksenzee and I got the library patch in.
Comment #2
berdirIs there anything that I need to do to get this commited?
Or can I assume that the patch is ok and will be committed once you find the time? :)
Comment #3
robloachAs a side note, CTools should update to use hook_library() and #attached instead of ctools_add_js() stuff.
Comment #4
merlinofchaos commentedThe patch is probably okay and will get committed.
Rob: #attached is only appropriate if using drupal_render(). I haven't bought into the render api and I haven't really spent any time converting things to. The area we're looking at isn't being rendered as part of drupal_render(), so I disagree with your assertion.
Comment #5
berdirOk, attaching a newer patch that fixes all other instances as well.
This is IMHO major due to the following problem:
It is possible that a late call to e.g. "drupal_add_js('misc/ajax.js');" overrides the previous definition and adds the file as JS_DEFAULT, at the end of the default list (e.g. if called in a preprocess function as in our case due to a call to ctools_ajax_text_button()). If there are then js files which define ajax commands, this results in Drupal.ajax is not defined for them and similar JS errors in IE which then prevent the JS from being executed.
Yes, I guess most uses of ctools_add_js() could probably be replaced by properly using the library pattern but I'd suggest to do that in a separate issue to get these fixes in asap :)
Comment #6
robloachComment #7
merlinofchaos commentedCommitted and pushed!
Comment #9
damontgomery commentedThere are still instances of ctools loading JS libraries directly instead of using the library when using $form['#attached'].
Two examples are ctools/plugins/export_ui/ctools_export_ui.class.php and ctools/page_manager/page_manager.admin.inc. Both use $form['#attached']['js'] = array('misc/jquery.form.js') and should use $form['#attached']['library'][] = array('system', 'jquery.form') as the views does.
I have not looked through everything or tested it, but I think this is important for the same reasons mentioned above. I also am having issues with jquery_update because of this.
Let me see if I can make a patch (I've never done this before).
Set-up:
Drupal 7.12, ctools 7.x-1.x-dev (Jan 23, 2012).
Comment #10
damontgomery commentedHere is the patch. Hope this is correct.
Edit: Use patch in Comment #13
Comment #11
damontgomery commentedSorry about the multiple comments, learning how to do this. Marking as Needs Review.
Comment #12
berdirCongrats to your first patch ;)
The patch should be relative to the module directory. (e.g. a/ctools/...) As you seem to be using git, this is easy to fix by using the --relative option to git diff.
git diff --relative=sites/all/modules or something like that.
The patch itself looks good to me.
Comment #13
damontgomery commentedThank you Berdir,
Here is an attached (fixed references) patch. Let's hope it works!
Comment #14
nod_:)
Comment #15
merlinofchaos commentedLooks good to me! Committed and pushed!