... even if new context is sdl_editor_representation.

This is caused by scald_atom_fetch_atoms() (in scald.pages.inc) suppling an empty (empty, not undefined) legend.

This can be fixed in three ways :

  1. scald_atom_fetch_atoms() does not provide any legend
    ==> scald.pages.inc : remove line 431 
            'legend' => '',
  2. scald_atom_fetch_atoms() provides same legend as scald_dnd_library_add_item() (in scald_dnd_library.module)
    ==> scald.pages.inc : add after line 434 
    if (empty($atom->omit_legend)) {
  $output[$sid]['meta']['legend'] = theme('sdl_editor_legend', array('atom' => $atom));
}
  3. dnd.js is patched to preserve atom's legend from being erased
    ==> replace line 78 
                var legend = Drupal.dnd.Atoms[atom_id].meta.legend;
            $.extend(true, Drupal.dnd.Atoms[atom_id], data[atom_id]);
            if (!empty(legend)) {
              Drupal.dnd.Atoms[atom_id].meta.legend = legend;
            }

First option is the most easy and clean, but I don't know of side effects...

CommentFileSizeAuthor
#6 scald-atom-legend-lost-2046545-6.patch420 bytesgifad
#2 scald-atom-legend-lost-2046545-2.patch420 bytesgifad

Comments

gifad’s picture

gifad commented
Title: Atom's legend is lost when changing context in "Edit items properties" » Atom's legend is lost when changing context in "Edit atom properties"

Edit : fix typo in title

gifad’s picture

gifad commented
Status: Active » Needs review
StatusFileSize
new scald-atom-legend-lost-2046545-2.patch420 bytes

maybe I should submit a patch (shortest patch to date...;)

jcisio’s picture

jcisio commented

Haven't tested, but I'm afraid that with this patch, changing into a context without legend (e.g. one that uses HTML5 image player) will preserve the legend.

In case this simple patch does not work: The legend is special thing that only exists inside SDL module. We should move it to Scald core instead.

gifad’s picture

gifad commented

Hi jcisio,

I took the time to track the whole life cycle of the “legend" :

It's created in theme_sdl_editor_legend(), which is invoked nowhere else than in scald_dnd_library_add_item();

Once dropped, it stays in the atom.meta.legend, is never seen by the server (except by the mee.module, which manage it as a “copyright”;

No player ever see it, even in the sdl_editor context.

It does not belong to the atom, it belongs to the textarea where the atom was dropped...

The concept of “context without legend” does not exist : the user may retype a new legend, after fetchAtom() erased it...

The scald_atom_fetch_atoms() function (which is invoked nowhere else than thru fetchAtom) should not send any legend, as it is never used to create atoms, but just to provide a new dnd-drop-wrapper div;

Finally, if a player provides a caption below the atom, and the site manager does not want to see a single second these “double legends”, he/she can make them exclusive via css (.context-{context} .dnd-legend-wrapper { display: none; })
At least, if/when the user selects a new other context, the “local” legend will re-appear..

If you “would move it to Scald core”, please keep separate
- the “canonical” legend (title+author)
- the “embedded local” legend (eventually adapted to the local context by the contributor)

Thanks for reading my rantings (google translate for élucubrations)...

jcisio’s picture

jcisio commented
Status: Needs review » Active

You hit the nail on the head. Legend is free text and one the atom is dropped, it can be customized by contributors, only in that specific text. Then came "player with native legend/caption (that can not be modified by contributor)". The idea here is to unify them, or make the player aware of "free text legend". Here is the plan (discussed during a code sprint) if I remember correctly:

Completely remove the dnd-legend-wrapper stuff, make the legend part of SAS option (like "link"). Then the player can choose to use custom legend if available, or the default one if not overriden. In that way, legend is kept when changing context. Should provide an update path in hook_update_N or an on-the-fly update (when editting a text field, convert old format to new one).

I'm not sure if you want to keep that in a new issue and push your patch #1 in this one (BTW patch #1 is broken), or simply orient this issue to that direction.

PS: copyright management in MEE is a straight port from D6. In D7 the whole markup (with the HTML comment) is editable and the copyright stuff is no longer relevant. Websites using Scald that I know often use a custom taxonomy for copyright because with fieldable entities in D7, it's easier to do that way.

gifad’s picture

gifad commented
StatusFileSize
new scald-atom-legend-lost-2046545-6.patch420 bytes

I don't understand “You hit the nail on the head.” : google translate talks about my finger on my head ???

Anyway, I completely subscribe to the quoted plan above : moving atom.meta.legend to options is the way to go.
A problem will arise if you agree the “contentEditable = true” feature : in case of “inline edit” of the legend, the plugin will not be fired, and the “ the legend part of SAS option” will not be set...

Attached the re-rolled patch about the current "bug";

Of course, discussions about the “legend” implementation should be another issue (once you have a formal proposition to submit..)

gifad’s picture

gifad commented
Status: Active » Needs review

forgot issue status

jcisio’s picture

jcisio commented
Status: Needs review » Needs work

As I explained in #3, the "preservation" of legend is incompatible with other players, like HTML5 player, because the legend ("caption" to be exact) is already configured in the back office, it makes atoms have double legends. But I can live with that (even after, it might make the upgrade path more difficult).

We'll need a change notice BTW.

PS: http://idioms.thefreedictionary.com/hit+the+nail+on+the+head

gifad’s picture

gifad commented

I disagree : caption and legend are not two words designing the same object, they are two different things :
- The HTML5 player's caption is a display of atom's title & author
- The legend is a description of the atom, in the context environment (page) where it has been dropped

the confusion comes from the fact that, in most cases, the caption is a good candidate for a legend...

all this IMHO, of course !

PS: You used the terms “back office”, and I strongly disagree with that back/front separation, at least since Xerox introduced wysiwyg on personal computers, +30 years ago; back office was introduced by Gutemberg ~600 years ago, That is old...

gifad’s picture

gifad commented
Status: Needs work » Needs review

We gone far off topic, I apologize for that;

The problem (“Atom's legend is lost ”) is about scald erasing valuable information entered by the user, without any “undo” possibility (the choice is either retype the legend, or cancel the whole form edit, and redo all previous edits).

If you do not agree with my proposed solution (at least for now), please provide a visual effect of the consequence of changing context : for instance erase the legend field of the dialog box;
The user will then be warned : he/she could cancel the dialog, reopen it, copy the legend, change context, paste the legend;

I thought it was better to erase the legend while changing the context if, with the new context, the legend would become irrelevant.

So please set the issue status to active or postponed...

jcisio’s picture

jcisio commented

I agree this is a bug. I did a quick test and the patch is good, but I want to leave it as "needs review" so that other can have a chance to give their thoughts.

Because this is a behavior change (even to a good direction), we need a change record at https://drupal.org/list-changes/scald.

ti2m’s picture

ti2m commented

Patch works for me and reflects the expected behavior, at least in my opinion.

jcisio’s picture

jcisio commented
Status: Needs review » Fixed

Committed d807f67. Thanks.

Status: Fixed » Closed (fixed)

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