Wysiwyg related plugins and functions should be in the mee module

Same issue here. After upgrading from Scald 7.x-1.4 to Scald 7.x-1.5 the node creation/edit form for all content types containing Atom Reference fields got broken. DND Library is enabled. Just downgraded to release 1.4 and everything is working fine again. FileField Sources was not used and is still not in use.

If you dont have it installed then we will need more information to reproduce the problem.
Its very likely that the core scald is not affected however and is more like a contrib module issue.

If you have a test site i can check the problem on, then i can help debugging it, but since it works on our sites, i cant do more without having more details.

Scald 7.x-1.5 testing (with DND library enabled): I just created a completely new content type. I deleted the body field to make sure it only contains the Node title and nothing else. I tried to add a new Atom Reference field. I got the same strange behaviour when entering the field settings (broken field edit form). Testing the same in Scald 7.x-1.4: field edit form is working. But: DND widgets are being loaded, even in the field settings form! This lets me guess there is a conflict between DND and something else.
I have the following contrib modules active which are hooking into the node edit form: Diff, Display Suite Extras, Organic Groups, Simplenews.

The dnd appears due the default value setting i guess.

But for me the form works fine.

OG can be ruled out, i have a site with OG and it works fine there.
So then it must be either display suite extras, or simplenews. Or some other module :)

Now I'm going to reproduce it with a fresh installation. Maybe I'll find out which contrib module conflicts with DND.

Sorry! I got the same behaviour on a fresh drupal-7.39 installation, downloaded from drupal.org. Enabled modules: Atom Reference, DnD Library, Scald Core, Scald DnD Library, Scald file, Scald image, Views.

After disabling scald_dnd_library on the test installation: node edit form works.

Sorry but i cant reproduce on simplytestme.

You can try to include screenshots, so far im out of ideas.

@kreatIL : You should enable the mee module, too, or install the latest dev version (this has been fixed recently)
To confirm this : you should have the following error message :
PHP Fatal error: Call to undefined function mee_store_format() in {...}/scald/modules/library/dnd/dnd.module on line 110
in your apache error log

Ok this is it. After I enabled mee the error went away. Thank you.

So I think Multimedia Editorial Element should be made a required module!! At least a dependand module of Atom Reference.

I don't think there is a need for a dependency here, as #2572585 was really a programming bug.
The real need is a scald 1.6, as a functional stable release...

FWIW, the attached patch moves some stuff from dnd.module to mee.module, where it's more rationally located

@DeFr can you review this patch?

While a competent person is at this, we could also move the CKEditor plugins to the mee folder;
I did it, and it works (once the paths have been adjusted);
Unfortunately, I'm not able to build a valid patch including this move /)

Fortunately, I'm still able to learn;=)

Well yes, i would like to have @DeFr's or @jcisio's validation before we do this, since its a lot of moving around.

Since this is not an issue anymore in dev, and the patch is related to restructuring I'm changing the task data.

missed the editor related css

re-rolled to current dev

@gifad: Not sure why the rename detection is not working for you (have you tried the -M option to git diff ?), but that sure makes the patch harder to read :-)

Uploading the same patch as #25, but with correct rename detections. I'll try to review the patch this afternoon.

Uploaded the wrong file above… Sorry about that.

I was also stuck with error: cannot apply binary patch to 'modules/library/dnd/plugins/dndck4/icons/atom.png' without full index line;
I solved it with git diff --full-index HEAD > ..

attached result of git diff -M --full-index HEAD > ../

This issue moves DnD plugins into MEE. I think it make a better separation between the library and the text field. A drawback is that MEE could work without DnD or JavaScript, but after some recent changes, MEE depends less or more on DnD now.

I don't have a strong preference for this issue. But this change is good to go as-is and does not have to wait for a bigger refactoring, which I think that won't happen at this stage.

MEE depends less or more on DnD

rather more than less : mee.info lists dnd as a dependency

MEE could work without DnD

This was true with SAS embedding method, but it does not make sense now with the embed_div method to create an embedded atom in another way than drag and drop...

@gifad: Well, as long as your field is set to be DnD enabled in the field config, you can enter the SAS representation manually in the edit form ( [scald=1:sdl_editor_representation] for example ), even with JS disabled, and get the atom to show on the page after the atom gets saved. Pretty sure that's still working, even if few users do it that way :-)

I do agree though that it makes sense to move the DnD plugins in MEE. DnD is for the library, MEE is the bridge between the library and the text fields, so having it in MEE makes sense ; less code to deal with for people that wants to only have Atom reference enabled.

I tested the upgrade path.

And it seems we need an update hook that resaves the ckeditor profiles. As otherwise until I resave the profile the system will try to search for the plugin at the wrong directory even after a drush cc all.

I tested the following snippet, inspired by ckeditor.module:ckeditor_modules_enabled()

if(module_exists('ckeditor')) {
  module_load_include('inc', 'ckeditor', 'includes/ckeditor.lib');
  $profiles_list = ckeditor_profile_input_formats();
  $plugins_list = ckeditor_load_plugins();
  foreach ($profiles_list AS $_profile => $_inputs) {
    $changed = FALSE;
    $profile = ckeditor_profile_load($_profile);
    if (!isset($profile->settings['loadPlugins'])) continue;
    foreach (array_keys((array) $profile->settings['loadPlugins']) as $plugin_name) {
      if (in_array($plugin_name, array('dnd', 'dndck4'))) {
        $changed = TRUE;
      }
    }
    if ($changed === TRUE) {
      db_update('ckeditor_settings')
          ->fields(array(
            'settings' => serialize($profile->settings)
          ))
          ->condition('name', $profile->name, '=')
          ->execute();
    }
  }
}

This actually invokes hook_ckeditor_plugin(), and should do the job;

Is it correct to include this code in mee.install ?

But i guess we will also have an issue if the wysiwyg module is used or not?
Maybe then its not so easy.

Update works fine while updating manually.
Not tested in real world conditions, though

[Edit] Cross posting during #37
I never used wysiwyg module, so yes, it won't be easy...
Leaving needs review for now, to pass the tests...

Unfortunately for me this update hook does not work :(

I still have to manually save the profile to get it to work.

Yes, I've been confused by some redundancies in ckeditor settings arrays...
Attached patch tested with :

• manually replace scald folder
• visit admin/reports/status, which detects need to updateDB
• click the suggested link

upate7003 is executed, and node/edit uses the right plugins, with no need to resave, clear caches, or whatever

About wysiwyg module, problem was inexistant : the wysiwyg module does not store, does not even cache plugins settings : hook_wywiwyg_plugin() is invoked at each ckeditor use...

Thanks!

It seems to work fine for me.

It will cause a feature overriden if the plugin configuration is stored in features, but that is probably okay, it often happens on module upgrades that certain features have to be recreated.

Thanks, Committed.