Found by @slybud while working on a customer project. Right now, there's a problem if you follow those steps using the DND CK4 plugin:

  1. Drag an atom into an RTE, go to the atom properties, untick the "Add a caption" box
  2. Save the entity
  3. Go to the edit edit form, right click the atom in the RTE, open the atom properties, tick the "Add a caption" box.

At that point, the expected result is to get the default caption showing up. That's not the case though, instead, a blank caption field gets shown ; it's a bit counter intuitive, and that caption field is really hard to access through CKEditor.

Working on a patch.

CommentFileSizeAuthor
#3 2496537-3-can-bring-back-the-legends.patch2.86 KBDeFr
#1 2496537-1-can-bring-back-the-legends-from-the-grave.patch2.74 KBDeFr

Comments

DeFr’s picture

DeFr commented
Status: Active » Needs review
StatusFileSize
new 2496537-1-can-bring-back-the-legends-from-the-grave.patch2.74 KB

That seems to work for me as a minimal fix.

That being said, the fact that there's 3 AJAX callbacks that tries to fetch extract some metadata about an atom, and that all of them fills Drupal.dnd.Atoms[sid] with slightly different datas seems both error prone and wastefull. Namely, there's

  • scald_atom_fetch_atoms
  • scald_dnd_library_add_item
  • mee_ajax_widget_expand

Status: Needs review » Needs work

The last submitted patch, 1: 2496537-1-can-bring-back-the-legends-from-the-grave.patch, failed testing.

DeFr’s picture

DeFr commented
Status: Needs work » Needs review
StatusFileSize
new 2496537-3-can-bring-back-the-legends.patch2.86 KB

Re-roll against latest dev.

nagy.balint’s picture

nagy.balint commented

Thanks for the patch.

However i found one issue on code review:
+ $output = $context ? scald_render($atom, $context, $options) : scald_render($atom, 'title');

But then we have

if ($context) {

So it seems that even though output will be set to the title render, it will never be returned as the context is still not evaluates to true.

DeFr’s picture

DeFr commented

It won't be returned, but it's still needed to get correct result from theme('sdl_editor_legend', as mentionned in the code comment above and in scald_dnd_library_add_item.

We probably should add a preprocess function to theme_sdl_editor_legend, that ensures that at least scald_prerender got called before trying to output the legend, but that feels a lot like scope creep that I'd rather avoid, and a source of potential issues in the future (right now theme_sdl_editor_legend in Scald core only relies on properties added in the prerender phase, but up to now we've always tried to give it a fully rendered one).

nagy.balint’s picture

nagy.balint commented

Okey then, its fine for me.

DeFr’s picture

DeFr commented
Status: Needs review » Fixed

Pushed as 3a5950e

  • DeFr committed 3a5950e on 7.x-1.x 
    Issue #2496537 by DeFr, nagy.balint: DNDCK4: Problem adding back a...

Status: Fixed » Closed (fixed)

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