In #769226: Optimize JS/CSS aggregation for front-end performance and DX, the drupal_add_js() API was changed to take JS_* constants as 'group' not as part of 'weight'. 'weight' and 'group' are now separate. Attached very simple patch to implement this.

CommentFileSizeAuthor
add_js_group.patch1008 bytesGábor Hojtsy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TwoD’s picture

Before this, wysiwyg.init.js was loaded after all the core scripts in misc/, with the patch it's loaded right after drupal.js and before vertical-tabs.js, states.js and form.js but given its contents this is not a problem.

Patch looks good and does what it's supposed to.

I'm wondering if we shouldn't also put the editor libraries in the JS_LIBRARY group as well (in wysiwyg_load_editor()). My initial tests with this seems to work fine, though I wonder what happens if a library decides to overwrite $ or something like that. I know openWYSIWYG does that so I made a quick test with it and CKEditor. Didn't notice any issues so far. (Wysiwyg's openwysiwyg.js resets the global $ back to jQuery and wraps openWYSIWYG's methods to use its version of $, but it's loaded long after openWYSIWYG's files.) Given the D7 JavaScript guidelines recommends using the (function ($){...})(jQuery); wrapper, this shouldn't be a problem for Drupal modules at least. Third party scripts not using such a wrapper might not be so fortunate.

Gábor Hojtsy’s picture

Well, this bug is about the basic API change that JS_* constants should now be used for 'group', not for 'weight'. Glad it tested well :) We run the module with this patch for the past two weeks, and it worked out for us nice as well.

sun’s picture

Status: Needs review » Fixed

Thanks for reporting, reviewing, and testing! Committed to HEAD.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Status: Fixed » Closed (fixed)

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