Steps to reproduce:
Create a custom content in panels, or IPE with panelizer.
Drag an image atom in the wysiwyg.
Right click and select edit atom properties.
The link field defined by the image provider is not there.

Likely the issue is that since the wysiwyg is loaded by ajax, the event that would add the extra fields does not fire on the new wysiwyg field.

CommentFileSizeAuthor
#3 extra-properties-in-ckeditor-from-ajax-2491245-3.patch4.41 KBnagy.balint

Comments

nagy.balint’s picture

nagy.balint commented

This can likely be an issue for other places as well, where there are no CKEDITOR on the page initially and it is only loaded on the fly in ajax.

nagy.balint’s picture

nagy.balint commented
Title: Panels custom content - extra atom properties does not work » Extra atom properties does not work when loading CKEditor in an ajax request without it being on the page initially
nagy.balint’s picture

nagy.balint commented
Status: Active » Needs work
StatusFileSize
new extra-properties-in-ckeditor-from-ajax-2491245-3.patch4.41 KB

The attached patch continues to make it easier to define new options in the edit atom properties dialog.

Also it fixes the issue by providing a timeout in case ckeditor is not yet loaded. It seems the issue was just a timing issue.

However i discovered that even if this works fine, the options do not work in the panelizer view of a custom content, so i still need to debug that.

Should be no backward incompatible change here, as I only defined new JS methods.

nagy.balint’s picture

nagy.balint commented

I found out why my custom content does not use the options in panels:

It is because in the widget implementation we store the options in an attribute on the div. And this function will remove that.
$content = ctools_context_keyword_substitute($content, array(), $contexts);
at
function ctools_custom_content_type_render($subtype, $conf, $args, $contexts) {
at
ctools/plugins/content_types/custom/custom.inc

nagy.balint’s picture

nagy.balint commented

Because the current code stores the options in a decoded form data-scald-options="%7B%22disable_credits%22%3Atrue%7D" for example. And that keyword function looks for the percentage sign.

Not sure about a solution at the moment.

nagy.balint’s picture

nagy.balint commented

An idea could be to grab a base64 implementation (since IE 9 does not support it natively)
And then we can replace the url encode with base64 encode, to provide a better markup without percentage signs.
We just have to make sure that if the string is not a base64 string then we use the url decode.

jcisio’s picture

jcisio commented

FYI I had the same problem when an atom is created quickly and not yet on the dnd library. I use a workaround to fix it as commenting here http://cgit.drupalcode.org/direct_upload/tree/plugins/ckeditor/direct_up..., because there is no AJAX request to do it.

Another problem was stated at https://www.drupal.org/node/2496537#comment-9969495.

So I think we should really focus to make a unified AJAX call to get all atom properties to replace all of our current AJAX calls. The dnd library is for displaying atoms, it is not its job to take care of extra atoms properties.

Back to the CTools keyword and the % character, wouldn't be there pre/post hook for keywork substitution? I haven't tested ctools yet, but will it replace if the keyword does not exist (e.g. '%22' is replaced even $keywords['22'] = NULL)?

nagy.balint’s picture

nagy.balint commented

I m not sure how the linked code is related to this issue, can you point it out?

This issue was just that the scald_image.js was loaded when the CKEDITOR variable was not even created yet, and so i made the code a bit better and also introduced a wait time in case that happens on ajax loading a ckeditor instance.

Also then discovered an issue with the custom content's keyword replacer, but that could be considered as a separate issue. But could be fixed here as well if its not too complicated.

nagy.balint’s picture

nagy.balint commented

And the issue we have is this one: https://www.drupal.org/node/935168

So this seems to be a ctools bug in fact.

I guess we could use base64 encode decode instead of url decode encode, and make sure its backward compatible with widget enabled sites. This is only a problem for widgets though.
(Edit: actually its maybe better to just try to push for getting the patch in ctools in the linked issue, see next comment.)

nagy.balint’s picture

nagy.balint commented
Status: Needs work » Needs review

Seems like the patch in the previously linked issue solves my problem, so we can make this needs review, and commit it, and users can patch their ctools with that one line patch to fix the other issue.

gifad’s picture

gifad commented

@jcisio :

we should really focus to make a unified AJAX call to get all atom properties

just a very small & simple step in this direction :

   $commands[] = array(
     'command' => 'dndck4_cache_atom_metadatadata',
     'data' => array(
+      'atom' => $atom,
       'sid' => $atom->sid,
       'meta' => array(
         'title' => $atom->title,
         ...

'meta'data could be removed, but problem is backward compatibility...

nagy.balint’s picture

nagy.balint commented

But i dont think this is related to this issue, can we move that discussion to a separate issue please?

nagy.balint’s picture

nagy.balint commented

I plan to commit this as this improves the API for providers to add their extra options to the dialog, plus it fixes an issue with any page that loads a CKEDITOR instance (not atoms) through AJAX.

If any objections, let me know :)

  • nagy.balint committed 0bc2cd3 on 7.x-1.x 
    Issue #2491245 by nagy.balint: Extra atom properties does not work when...
nagy.balint’s picture

nagy.balint commented
Status: Needs review » Fixed

I don't see an issue with this patch, and it improves the API by letting providers define only the code for the option, and not need to bother with anything else. Plus it fixes the issue for any ajax environments like paragraph module or ctools modals where a ckeditor instance is loaded in ajax (again not the atoms), and where a ckeditor instance did not exist on the page before the ajax load.
Plus it is backward compatible as it keeps the previous way of implementing an extra option intact.

Committed.

Status: Fixed » Closed (fixed)

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