The insert module does some file inclusion in hook_init().
These files contain the hook implementations for hook_insert_widgets() and hook_insert_styles().

At least for hook_insert_widgets(), it can happen that the module_implements() is called before insert_init(), and the result is stored in a static variable.
One example is dqx_adminmenu, which builds a form at hook_init() time, which then triggers insert_widgets().

Now, while this could be seen as my fault (being the author of dqx_adminmenu), it is generally better to assume that anything can happen before insert_init(). In my experience, hook_init() is just not a safe place for anything.

Going to upload a patch asap..

CommentFileSizeAuthor
#1 insert.d6.hook_init_too_late.patch1.27 KBdonquixote

Comments

donquixote’s picture

Status: Active » Needs review
StatusFileSize
new1.27 KB

This patch does make sure that the respective files are included before any module_implements().

I think the performance footprint should be close to zero, as _insert_include_once() has a $_first_run static, and insert_styles() and insert_widgets() both have a static cache.

patrickroma’s picture

Priority: Normal » Major

With dqx_adminmenu and this patch the insert widgets are shown again, but the url's are not printed in the body-field after insert as without the dqx enabled =>
Without menu:data-mce-href="http://dev.89.0rtl.de/sites/default/files/example_1.jpg"
With the dqx the image src is not printed.

donquixote’s picture

Do you notice any erros in the js console?
What version of dqx adminmenu?

Btw, I think newer versions of dqx no longer cause this kind of conflict, because they use hook_pageapi() instead of hook_init().

Still it is probably a good idea to commit this patch nevertheless, just to make this thing more robust.

gaëlg’s picture

Status: Needs review » Reviewed & tested by the community

Patch #1 worked for me. And I didn't notice any problem with the src (I'm using CKEditor module), neither any js error.

quicksketch’s picture

Status: Reviewed & tested by the community » Closed (won't fix)

I think this is just a case of "too bad". :P

hook_init() should be early enough for all situations, but we can't use module_exists() in places where Drupal isn't fully bootstrapped (since it requires a database connection). Considering hook_init() is the first hook that has a database available, I don't think we have other good options.

If dqx adminmenu needs to run in hook_init(), it should set its module weight so that it runs AFTER other modules have run hook_init() and done their inclusions. Right now starting with "d", dqx is going to be executed before "insert", but sounds like that's not needed any more anyway so the whole thing is moot.

donquixote’s picture

First of all: I don't care as much anymore, because dqx no longer uses hook_init().

However: I think the patch in #1 is actually quite solid. The major use case would be if another module's hook_init() fires before insert_init(). So we already do have the database, np.
I leave it with this comment, so if any time you feel like it, you can grab that patch and commit. And if not, I won't make any further noise.

(this said, I think the db conn would be there in hook_boot already. Only the module system is not fully set up at this time, afaik. But this is not relevant, because patch in #1 does not use hook_boot() anyway)