The "edit" action in the atoms administration page (admin/content/atoms) the link ends in "/nojs" (for example atom/123/edit/nojs). This link will take the user to the atom edit page, but no local actions (View, Edit, Translate, Delete) etc are visible. I guess this is because of the menu router not being able to figure out the current location in the menu tree because of added "nojs" path.

I know this is becuase the content adminstration page per default uses the same atom representation as the dnd-library (sdl_library_item), but as the ctools-modal is not loaded here, "nojs" and the ctools-modal classes are not needed.

Somehow "scald_atom_user_build_actions_links" needs to be context aware, and format the links differently depending of context. I don't think it is a good idea to use the modal in the adminstration page since it's nice to be able to fall-back to a "standard" drupal editing interface. Besides the "translate" link is not visible in dnd-library editing mode etc, so right know I depend on being able to use admin/content/atoms for that purpose.

I have tried searching trough the issue queue but found no one else with this problem, so I hope this is reproducible, and that I have not made some configuration error or other mistake.

Meanwhile I will look into this and try to figure out some suggestion how to correct this. New special atom representation for the view? Probably not since an alternative "scald_atom_user_build_actions_links" will have to be used to generate the links. Provide more context for "scald_atom_user_build_actions_links" and hook_scald_atom_user_build_actions_links could be an idea though.

CommentFileSizeAuthor
#9 scald-links-nojs-2619818-9.patch5.87 KBgnucifer
#8 scald-links-nojs-2619818-8.patch5.87 KBgnucifer
#7 scald-links-nojs-2619818-7.patch5.84 KBgnucifer
#3 scald-links-nojs-2619818-3.patch969 bytesgifad

Comments

gnucifer created an issue. See original summary.

gnucifer’s picture

gnucifer commented

Perhaps the $options for the sdl_library_item context can be used for this, I will try to work out a patch.

gifad’s picture

gifad commented
Status: Active » Needs review
StatusFileSize
new scald-links-nojs-2619818-3.patch969 bytes

The attached patch moves the "/nojs" addition from the generic scald.module generic implementation to the dnd-library.js render library, which is the only place where it's needed.

gnucifer’s picture

gnucifer commented

Isn't the whole purpose of "nojs" to indicate server side whether or not the request was made through ajax or not (if client has javascript enabled or not)? If so you cannot add it by javascript as that would defeat the purpose. At least in the drupal ajax framework it is stripped on ajax submit, but I'm not sure how the "nojs" comes into play in scald. If it is a a scald thing, a ctools modal thing, or a drupal ajax framework thing.

gifad’s picture

gifad commented

This patch is just about html generation : it operates before anything is submitted.
Please try it, and report any unattended side effect...

gnucifer’s picture

gnucifer commented

To clarify my previous response: yes, the patch probably will fix the the problem, but it does so in a confusing way. When clicking a link with "/nojs/" in the path, ctools will replace "/nojs/" with "/ajax/". This so that the page callback in Drupal can determine if the call was made through ajax or not. To append "/nojs/" in confusing, because "/nojs/" is supposed to be a placeholder for "/ajax/" when javascript is NOT enabled. We might as well append "/ajax/" in that case, to make it a little more clear what we are doing.

I think the fundamental problem is that the context "sdl_library_item" is used in the scald_atoms admin view. scald_dnd_library is not a dependency for scald, and still this context (which is defined in scald_dnd_library.module) is used in scald.module. I think instead that the scald admin view should use another context, perhaps some kind of "teaser"-context for the atom representation, plus the actions links minus the ajax-stuff. I will try to submit a patch to showcase this suggestion.

gnucifer’s picture

gnucifer commented
StatusFileSize
new scald-links-nojs-2619818-7.patch5.84 KB

The patch moves scald_dnd_library stuff in scald.module to scald_dnd_library.module and uses a new dedicated scald context for atom representation in the scald_atoms (admin/content/atoms) view.

gnucifer’s picture

gnucifer commented
StatusFileSize
new scald-links-nojs-2619818-8.patch5.87 KB

Fixed accidental overwriting of link attributes (target="_blank").

gnucifer’s picture

gnucifer commented
StatusFileSize
new scald-links-nojs-2619818-9.patch5.87 KB

Rerolled against current HEAD.

ciss’s picture

ciss commented

@gnucifer While I agree that it's somewhat pointless to add a "/nojs" suffix with Javascript (even if it's just to have it replaced), I feel like the patch provided by yourself introduces too many changes that are most likely breaking backwards compatibility for existing customizations.

I'd propose to keep the changes to a minmum by improving on the patch in #3 according to the suggestions in #5. In my opinion the issue raised in #6 (sdl_library_item context used for different contexts - pun intended) should be dealt with in a separate issue.

ciss’s picture

ciss commented
Status: Needs review » Needs work

In addition to what I wrote in #10, all patches are ignoring EntityTranslationScaldHandler::getEditPath() where nojs is still included.

ciss’s picture

ciss commented
Related issues: +#2882778: Atom view/edit/delete links open in a new tab, +#2325585: Custom 'edit path' breaks local actions (translation tabs)
ciss’s picture

ciss commented
Related issues: +#2662494: Translating an atom gives a warning and a notice
ciss’s picture

ciss commented

I'm happy to say that the problem can be solved quite easily by using MENU_DEFAULT_LOCAL_TASK and tab_parent.
For now we've implemented this via hook_menu_alter(), but it can easily be applied to scald_menu():

function hook_menu_alter(&$items) {
  $tab_map = array(
    'atom/%scald_atom/edit/%ctools_js' => 'atom/%scald_atom/edit',
    'atom/%scald_atom/delete/%ctools_js' => 'atom/%scald_atom/delete',
  );
  foreach($tab_map as $child => $parent) {
    $items[$child]['title'] = $items[$parent]['title'];
    $items[$child]['type'] = MENU_DEFAULT_LOCAL_TASK;
    $items[$child]['tab_parent'] = preg_replace('_%[^/]+_', '%', $parent);
  }
}