[legacy ckeditor plugin] Default options from library alter using insert method will not carry over

I can see two options:
- Override the scaldEmbed theme function.
- Extend (patch) Scald so that data returned in Drupal.dnd.fetchAtom can contain a default "options" (hook_scald_atom_default_options(_alter)?) and modify the scaldEmbed theme to use it if it is available.

I started to do an implementation of hook_scald_dnd_library_item_alter but there seems to be an issue. It looks like the only place sas2array is called is on the drop event, so the new "Insert" and double-click options for inserting into the ckeditor dom don't take into consideration any library item modifications. I may be doing something wrong, but it works fine on dnd, but not on insert or double-click. Is there a simple way to patch the plugin.js file to consider the clicking of the insert link?

Filing as a bug because this seems to be a failure of that hook to consider the insert and double-click options that were added to core.

Not sure if it does or not, but I'll check it out. In the meantime, I worked out that insertAtom doesn't consider the options parameter of scaldEmbed. I just overrode insertAtom to run the sas through sas2array, get the options (altered by hook_scald_dnd_media_library_alter()).

    insertAtom: function(sid) {
        var cke = Drupal.ckeditorInstance;
        if (cke) {
          var context = Drupal.dnd.sas2array(Drupal.dnd.Atoms[sid].sas).context;
          var options = Drupal.dnd.sas2array(Drupal.dnd.Atoms[sid].sas).options;
          var markup = Drupal.theme('scaldEmbed', Drupal.dnd.Atoms[sid], context, options);
          cke.insertElement(CKEDITOR.dom.element.createFromHtml(markup));
        }
        else if (Drupal.dnd.lastFocus) {
          var markup = Drupal.dnd.Atoms[sid].sas;
          Drupal.dnd.insertText(Drupal.dnd.lastFocus, markup);
        }
        return true;
      }

Just in case the use case is useful to anyone else, here's our implementation of hook_scald_dnd_media_library_alter(), linking the image atoms to their raw full-size image file instead of the atom page as described above.

/**
 * Implements hook_scald_dnd_library_item_alter().
 */
function scald_overrides_scald_dnd_library_item_alter($atom, &$library_item) {
  if ($library_item['meta']['type'] == 'image') {
    $rawImagePath = file_create_url($atom->base_entity->uri);
    // We'll default all image atoms to link back to their raw image
    // Do this by updating the atom's sas to have a "link" option
    if (preg_match("/\[scald=(\d+)\:([^\s]+)?(.*)]/", $library_item['sas'], $matches)) {
      $library_item['sas'] = '[scald=' . $matches[1] . ':' . $matches[2] . ' {"link":"' . $rawImagePath . '"}]';
    }
  }
}

Bump. Any chance we can confirm or deny that this is a deficiency of Scald or just something about the implementation I've described?

It looks like the only place sas2array is called is on the drop event, so the new "Insert" and double-click options for inserting into the ckeditor dom don't take into consideration any library item modifications...this seems to be a failure of that hook to consider the insert and double-click options that were added to core.

If this was fixed/changed in 1.3, we haven't gotten around to updating yet but will consider that as an option, especially if it addresses this issue. Any feedback on this is appreciated.

If you mean comment #7, that is only available in the dev version.

We have since realized that, in working towards upgrading to 1.4, our project has far too much code that is dependent on the sas markup to consider moving forward with the dndck4 plugin. Any way that this could be more permanently fixed so that sas2array is called on other events besides drop? Thankfully, you guys have been conscious of the need for the sas markup to continue to work, so for that I'm very grateful! :)

Unfortunately the old dnd plugin is in legacy status right now, which means that of course if someone can provide a patch, and we can get confirmation that it works, or if there is some serious bug in the old system, then we are open to commit the patch, or to fix the serious issue. However it is unlikely to receive any new feature development.

On the other hand if you can describe the code that depends on the sas markup, then maybe we can figure out a way to still upgrade to the new version.

At any rate i would still recommend upgrading to the latest version even if you keep using the legacy dnd plugin.

Sorry for reviving this issue in the first place, as it looks like this is fixed in 1.4 anyway! Before, only if I dragged and dropped would I get the custom implementation of library_item_alter with the link but a test with 1.4 revealed all 3 actions working the same as they should! Thx!

No, definitely not fixed. After some extensive testing, there is still something definitely wrong with the way this is working. If I use hook_scald_dnd_library_item_alter() as mentioned above to add a link to the default sas markup, drag and drop works fine but Insert and Double-click do not. I cannot seem to figure out why that is. I will continue to investigate, but to me this constitutes something fundamentally broken that has never been addressed. It shouldn't matter what method you use to insert the atom into the RTE, the use of the hook should be considered in all 3 cases.

And that refers back to comment #7 I believe.

I would still suggest to consider more to upgrade to dndck4, as the legacy plugin will only receive legacy support, which is not too much at the moment. Of course if you can provide a patch, i will be happy to review it.

Unfortunately for our site "upgrading" is, for many reasons, just not possible right now (mainly that all our custom code breaks as soon as I tried!). I would happily provide a patch but I can't seem to nail down where the difference is. Can you look and help me figure out where the discrepancy is in how the 3 methods handle things? I think it boils down to scaldEmbed not being passed the options array when it's first run using insert or double-click but it IS passed the options when you DnD.

Isnt it possible for you to describe what custom code breaks when you try dndck4?

Cause on our sites, there was never any custom code that depended on the sas code.

Do you have some custom filters that cannot be replaced?

In a nutshell, the main thing that breaks is a custom module that lets the user specify a caption, credit and link (title and url) and this makes up the value of the legend field without actually giving them access to the legend field. I guess it's primarily because we're using custom versions of the onShow and onOk methods from the atomProperties dialog box.

Regardless, let's focus on the point at hand if we can: the method for inserting an atom into CKEditor should not matter. It should (and always should have) consider(ed) the fact that a library item can be altered using a Scald API hook (hook_scald_dnd_library_item_alter) and that when it is inserted into the textarea it may already have options on it that need to be considered. DragNDrop handles this nicely, Insert and Double-click do not.

I will continue figuring out why because I can't spend the time to try and upgrade this site to use the new plugin; the size and scale of this site would help you understand if I could say which one and get into the details, but I can't! If I could, the Scald team would have a really nice one to brag about on the project page! ;)

Seems like this does the trick for us. Don't know if it breaks other stuff yet...

Not sure, but I think the inconsistency comes from the fact that the drop event was the only one which ran things through the sas2array to get the options and then does a fetchAtom, runs that through scaldEmbed theme function with the context and options, then insertElement into the editor. The other 2 events are just calling insertAtom, which doesn't look at options at all...something like that anyway. :)

This patch should be fine.

Thanks, Committed.

Thanks for the moral support. :) I knew I could figure out a way to fix it if I stuck with it.