Using the latest version of linkit (dev) and I noticed that the module declares hook_ckeditor_plugin() to instantiate its config and button. This also seems to be the case with a recent version of ckeditor_link.

Looking at http://drupalcode.org/project/ckeditor.git/blob/refs/heads/7.x-1.x:/incl...

I see that external plugins are declared here. And this is actually breaking Linkit integration for us. This is due to the double-declaration of plugins - first by the module (ie: Linkit) and then by the CKEditor module. The result is that the values for 'name', 'path' and so on end up being arrays with double-casted values.

Think $plugin['path'] = array('/path/to/linkit/plugindir/', '/path/to/linkit/plugindir/'); instead of $plugin['path'] = '/path/to/linkit/plugindir/';

When the CKEditor settings form is built (and likely in other places) there are checks in place to ensure that things like 'plugin.js' can be found.
See: http://drupalcode.org/project/ckeditor.git/blob/refs/heads/7.x-1.x:/incl...

It seems the best resolution here would be for CKEditor to step aside from declaring any external plugins. Instead, it should let the modules do this. The patch attached simply removes these redundant plugin declarations.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

saltednut’s picture

I imagine some people may want to use this patch with the changes from #1504696: Integration with CKEditor module

The attached patch should apply and work the same way if you've already applied https://drupal.org/files/issues/ckeditor-accomodate-latest-media-changes...

saltednut’s picture

Status: Active » Needs review
jcisio’s picture

Status: Needs review » Needs work
Issue tags: -demo_framework

Do you have links on the other projects where changes went in? Since which version?

The filename in the patch looks wrong btw.

saltednut’s picture

Status: Needs work » Needs review
Issue tags: +demo_framework

We are using this in the demo framework, so please don't remove the tag. It is used so that we can track issues for the distribution more easily.

Linkit 3.x uses hook_ckeditor_plugin(): http://drupalcode.org/project/linkit.git/blob/db68789:/linkit.module#l969

Could not find the exact commit for 2.x but the latest HEAD of ckeditor_link for both 7.x-1.x and 7.x-2.x use hook_ckeditor_plugin()
ckeditor_link 7.x-2.x - http://drupalcode.org/project/ckeditor_link.git/blob/refs/heads/7.x-2.x:...
ckeditor_link 7.x-1.x - http://drupalcode.org/project/ckeditor_link.git/blobdiff/184fdfe7a487826...

ckeditor_swf: http://drupalcode.org/project/ckeditor_swf.git/blob/refs/heads/7.x-1.x:/...

See comment #1 to answer why the filename in the second patch varies.

jcisio’s picture

Sorry for the tags, I don't understand the "demo framework", but you can of course keep them.

I follow those commits, and:
- LinkIt 3.x has it committed in June 2013.
- CKEditor Link has it committed (in both 1.x and 2.x) in June 2011 (sic) https://drupal.org/node/1179916
- The same for CKEditor SWF: June 2011 https://drupal.org/node/1179918

I don't understand why nobody noticed it. I don't have time to investigate now (but want to give a quick reply), so we need to either find the problem why it was going unnoticed in more than 2 years or move these 3 plugins into hook_ckeditor_plugin_alter (after check that they are not declare by 3rd modules) to be safe.

jcisio’s picture

SocialNicheGuru’s picture

conflicts with path needed for ckeditor to work with media: [2159403]

saltednut’s picture

Updated alt patch from #1. Must be applied after the patch from #2159403-23: Make CKEditor plugin system modular and clean

Status: Needs review » Needs work

The last submitted patch, 8: ckeditor-remove-external-plugin-declarations-8-alt.patch, failed testing.

jlyon’s picture

#8 solved the issue for me. Thanks!

jcisio’s picture

Status: Needs work » Postponed
saltednut’s picture

Issue tags: +Needs reroll

This needs a reroll.

agoradesign’s picture

Hi guys,
I have worked on Make CKEditor plugin system modular and clean because I also had troubles with CKEditor Link plugin after applying the existing patch from that issue. I have updated the patch and removed all the external plugin declarations like you did. Additionally, I had to correct their paths in the ckeditor_plugin_alter hook. Why didn't you need that for your patch?

Nevertheless, it would be nice, if you could try my patch, so that we can get a step closer to have a working Media integration

othermachines’s picture

To clarify, #8 patch has been rolled into latest patch for Issue #2159403: Make CKEditor plugin system modular and clean.

Should this be closed as duplicate?

jcisio’s picture

Status: Postponed » Closed (duplicate)

Yes. In #2159403: Make CKEditor plugin system modular and clean we've fixed all plugin declarations on behalf of other modules.