I've created a patch to move the requirement-check of "clean urls" from hook_init to hook_requirements.
Because hook_init is called every page-load I moved this to hook_requirements. When enabling the module, you get an error-message when Clean urls is not enabled.

Comments

anrikun’s picture

Yep, but what happens if user disables Clean URLs afterwards?
Does it work too?

Edit:
After looking at your patch, it seems that you didn't move but added things, right?

nico heulsen’s picture

StatusFileSize
new2.52 KB

Good point, didn't keep that in mind. But I really think we should avoid the hook_init as much as possible, so I created a new patch (in this I removed the hook_init), and added an extra submit-handler (on the Clean URLs settings form) to disable the CKEditor link module when disabling the "Clean URLs". I also added a notice within the description to inform the user.

anrikun’s picture

Status: Needs review » Needs work

Thanks! Several problems though:
- Your patch does not apply to the last dev. I can see this because of a missing menu_rebuild();
- ' <em>(' . t('Notice : When disabling the "Clean URLs", CKeditor Link module will also be disabled.') . ')</em>' needs a better message, and no ems!
- drupal_set_message(t('Disabled CKEditor module, since it requires "Clean url" to be active.'), 'notice');: message is wrong and this should be a warning IMHO.

devin carlson’s picture

Title: Move requirement of clean urls from hook_init to hook_requirements » Move Clean URL requirement from hook_init to hook_requirements
Category: feature » task
Status: Needs work » Needs review
StatusFileSize
new1.45 KB

This is still an issue.

Besides the performance impact, using hook_init makes it very difficult to automatically configure CKEditor Link using an installation profile because the module automatically disables itself whenever it detects that Clean URLs aren't available (and Clean URLs aren't available during installation).

I've provided an updated patch against 7.x-2.x-dev.

anrikun’s picture

Status: Needs review » Fixed

Thanks to both of you!
This is now committed to 6.x-2.x-dev and 7.x-2.x-dev with some minor changes/fixes.

Status: Fixed » Closed (fixed)

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