Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.mollom.js only contains functions used with Drupal.behaviors. These are not executed until the whole page has loaded, so there is no reason to load the file already in the <head> element.
Also, AFAICT mollom.js is not used in mollom.admin.inc.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | mollom-footer-rollback-D6.patch | 1.41 KB | c960657 |
| #11 | mollom-footer-rollback-D7.patch | 1.15 KB | c960657 |
| #3 | 781994-mollom-footer-js-D7.patch | 704 bytes | dave reid |
| mollom-footer-1.patch | 991 bytes | c960657 |
Comments
Comment #2
dries commentedCommitted to DRUPAL-6--1 -- the test failure has nothing to do with this patch. Needs to be ported to Drupal 7.
Comment #3
dave reidHere's the same patch for 7.x, and committed to CVS.
Comment #4
dave reidActually I'd love just to see how tests are doing in D7. :)
Comment #6
dave reidComment #7
sunI'm a bit baffled by this change. The stated reason for loading the JavaScript in the footer was
But given that intention, changing the scope is the wrong optimization. Instead, it's the
deferattribute that would have to be added for mollom.js.Merely changing the scope can easily lead to the opposite of the intention (assuming frontend performance): mollom.js is no longer aggregated if JS aggregation is enabled.
Overall, you'd have to apply the same scope change to almost every other module and JS file that runs on your site. I am well aware of the fact that loading JS in the footer (if possible) is a known frontend performance optimization. But if you really want to apply it globally, then we should change the default scope in Drupal core instead, and users of current core versions may rescue to contributed modules that allow to configure this optimization.
Comment #8
c960657 commenteddefer would be better, at least with modern browsers. But can we be sure that behaviors are attached if the deferred file finishes loading after the $(document).ready() is fired?
Scripts inserted into 'footer' are aggregated too. But aggregation requires that at least one other JS file is inserted here.
After having files this issue I noticed that Drupal core inserts scripts using the defaults a lot of places where scope == 'footer' or 'defer' => TRUE could have been used instead. I have just filed #784626: Default all JS to the footer, allow asset libraries to force their JS to the header.
True. I hope we can decide on a general recommendation for this in #784626: Default all JS to the footer, allow asset libraries to force their JS to the header that other modules may follow. But for now this change is perhaps too “avant garde” for Mollom.
Comment #9
sunadmin_menu successfully uses the defer attribute since Drupal 5, so yes, it works how it's supposed to work. And, yes, most JS in Drupal could technically use the defer attribute.
Comment #10
dave reidShould we roll this back? It doesn't seem like a worthwhile change especially since it can't be aggregated now.
Comment #11
c960657 commentedAgreed. We should roll it back until a conclusion has been reached in #784626: Default all JS to the footer, allow asset libraries to force their JS to the header.
Sorry for jumping the gun on this. It was my impression that 'footer' was already widely used.
Note that the D6 patch does not roll back the change to mollom.admin.inc. That change is unrelated to the header/footer discussion.
Comment #12
dave reidComment #13
dries commentedRolled back in DRUPAL-6--1 and CVS HEAD. Back to 'fixed', I guess.