Using Media Browser Plus 7.x-2.x-dev I was unable to delete media files from the browser interface. The deletion confirmation page listed no files, and clicking the "Delete" button to confirm produced this:

Undefined index: files in file_multiple_delete_form_submit() (line 432 of .......sites/all/modules/file_entity/file_entity.pages.inc).

It turned out to be an issue with file_entity's file_entity_multiple_delete_confirm_operation function on line 837 of file_entity.module:

function file_entity_multiple_delete_confirm_operation($fids) {
  ...
}

The $fids variable was always empty. However when I changed it to this:

function file_entity_multiple_delete_confirm_operation($form, $form_state, $fids) {
  ...
}

then everything worked fine. I'm not too sure what the procedure is now with patches (whether it's all about the diff files or git patches) so I've wussed out & left it alone :)

Comments

dave reid’s picture

Project: File Entity (fieldable files) » Media Browser Plus

I'm unable to reproduce this issue with just File entity installed, so it must actually be Media Browser Plus that is causing the issue.

Sunlight Studios’s picture

I am not sure which other modules use this functionality apart from Media Browser Plus, so I can't test it with anything else. However Media Browser Plus uses it in this context:

...
  $form = drupal_get_form('file_entity_multiple_delete_confirm_operation', $files);
  return $form;
...

If file_entity_multiple_delete_confirm_operation is indeed designed to be used with drupal_get_form then it should include $form and $form_state in its parameter list - see http://api.drupal.org/api/drupal/includes%21form.inc/function/drupal_get... (or indeed other functions called by drupal_get_form in the core code)

I am assuming this is the case so I'm bouncing it back to the file_entity project, if I've got it totally wrong then plonk it back to Media Browser Plus & slap me :)

Sunlight Studios’s picture

Project: Media Browser Plus » File Entity (fieldable files)
dave reid’s picture

Project: File Entity (fieldable files) » Media Browser Plus

The form ID changed in 7.x-2.x for deleting multiple files and needs to be fixed in the branch of Media Browser Plus which is compatible with Media 2.x only.

module_load_include('inc', 'file_entity', 'file_entity.pages');
$form = drupal_get_form('file_multiple_delete_form', $files);

Note that the $files needs to be an array of actual file objects from file_load_multiple() and not an array of just file IDs as well.

viggo’s picture

Status: Active » Needs review
StatusFileSize
new788 bytes

I've created a patch from what Dave Reid wrote in comment #4.
Deleting multiple files now works for me.

emptyvoid’s picture

I am having problems with both list and thumbnail deletion..

more there: http://drupal.org/node/1679604

viggo’s picture

Try doing what Dave Reid describes in #4.
Or you can even check out the patch in #5.

Notice that the method referenced should be file_multiple_delete_form, and not file_entity_multiple_delete_confirm_operation.
Also, when referencing file_multiple_delete_form, files must be loaded with file_load or file_load_multiple.

emptyvoid’s picture

Unfortunately file_entity changed the method names and method descriptor to include a different set of parameters. So the methods don't interact correctly any longer.

The target method:
/modules/file_entity/file_entity.pages.inc

/**
 * Form constructor for file deletion confirmation form.
 *
 * @param array $files
 *   An array of file objects.
 *
 * @todo Rename this form to file_delete_form to ease into core.
 */
function file_multiple_delete_form($form, &$form_state, array $files) {
  $form['files'] = array(
    '#prefix' => '<ul>',
    '#suffix' => '</ul>',
    '#tree' => TRUE,
  );

  $files_have_usage = FALSE;
  foreach ($files as $fid => $file) {
    $title = entity_label('file', $file);
    $usage = file_usage_list($file);
    if (!empty($usage)) {
      $files_have_usage = TRUE;
      $title = t('@title (in use)', array('@title' => $title));
    }
    else {
      $title = check_plain($title);
    }
    $form['files'][$fid] = array(
      '#type' => 'hidden',
      '#value' => $fid,
      '#prefix' => '<li>',
      '#suffix' => $title . "</li>\n",
    );
  }

  $form['operation'] = array(
    '#type' => 'hidden',
    '#value' => 'delete',
  );

  $description = t('This action cannot be undone.');
  if ($files_have_usage) {
    $description .= ' ' . t('Some of the files are currently in use and may cause problems if deleted.');
  }

  return confirm_form(
    $form,
    format_plural(count($files), 'Are you sure you want to delete this file?', 'Are you sure you want to delete these files?'),
    'admin/content/file',
    $description,
    t('Delete')
  );
}

As you see here the method expects 3 parameters not 1.

I'll spend some time and step through each method to determine what objects/arrays are required for the method to properly target and delete the entity record and media file. I'll post my research notes as I refactor the module.

I should mention that the delete on the list view and the thumbnail view don't work at all (I'd assume both use the base base functions for deleting the entity and media file). At least that is my hope, if I fix the main methods then all GUI displays should be fixed (crosses fingers).

emptyvoid’s picture

Ok, that's crazy.

So on a lark I redownloaded the 2.x-dev branch and applied the patch as mentioned in #5. Now file deletion is possible on the "thumbnails" display however the "list" display still errors.

On submission of one or more files the confirmation screen displays:

Notice: Undefined index: options in _media_browser_plus_media_admin_list_alter() (line 192 of /srv/clients/example/sites/all/modules/contrib/media_browser_plus/includes/file_entity.admin.inc). Backtrace:

_media_browser_plus_media_admin_list_alter(Array, Array) file_entity.admin.inc:29
media_browser_plus_form_file_entity_admin_files_alter(Array, Array, 'file_entity_admin_files') module.inc:1022
drupal_alter(Array, Array, Array, 'file_entity_admin_files') form.inc:1067
drupal_prepare_form('file_entity_admin_files', Array, Array) form.inc:475
drupal_rebuild_form('file_entity_admin_files', Array, Array) form.inc:924
drupal_process_form('file_entity_admin_files', Array, Array) form.inc:374
drupal_build_form('file_entity_admin_files', Array) form.inc:131
drupal_get_form('file_entity_admin_files') 
call_user_func_array('drupal_get_form', Array) menu.inc:516
menu_execute_active_handler() index.php:21

I can however submit via the "delete" button.. doing so deletes the selected file, however I get the following error on the index page.

Warning: Invalid argument supplied for foreach() in form_select_options() (line 2656 of /srv/clients/example/code/includes/form.inc). Backtrace:

form_select_options(Array) form.inc:2631
theme_select(Array) theme.inc:1060
theme('select', Array) common.inc:5766
drupal_render(Array) common.inc:5773
drupal_render(Array) common.inc:5773
drupal_render(Array) common.inc:5839
drupal_render_children(Array) system.module:3960
theme_confirm_form(Array) theme.inc:1060
theme('confirm_form', Array) common.inc:5766
drupal_render(Array) common.inc:5773
drupal_render(Array) common.inc:5865
render(Array) page.tpl.php:28
include('/srv/clients/example/code/themes/seven/page.tpl.php') theme.inc:1413
theme_render_template('themes/seven/page.tpl.php', Array) theme.inc:1103
theme('page', Array) common.inc:5766
drupal_render(Array) common.inc:5629
drupal_render_page(Array) common.inc:2583
drupal_deliver_html_page(Array) common.inc:2471
drupal_deliver_page(Array, '') menu.inc:531
menu_execute_active_handler() index.php:21

I'll review code and try to track down the nature of the warnings.

Jackinloadup’s picture

As stated above patch in #5 get thumbnail view working but not list view.

emptyvoid’s picture

StatusFileSize
new33.22 KB

The first error is a two part issue.

1) Confirm Display
I believe the display template being called isn't the correct display expected.

media list delete confirm

You will notice I highlight GUI elements I think do not belong in a deletion confirm display. The operations element above is what is causing the warning error to occur.

The line that causes the error to occur is below.

media_browser_plus/includes/file_entity.admin.inc

192: $ops = $form['options']['operation']['#options'];

Basically this is targeting a multi-dimensional array of values that should be used to render into the drop down of the "Operations" form. Clearly this form and the footer should not be on this page.

As I mentioned before clicking the "Delete" button does delete the file. So the operation still occurs as desired.

emptyvoid’s picture

The following removes the display of the header and footer on the list page.

Current code

/**
 * Alter the media list display.
 *
 */
function _media_browser_plus_media_admin_list_alter(&$form, &$form_state) {
  global $user;

  // Check access rights.
  if (!media_browser_plus_access('media list view')) {
    drupal_access_denied();
    return array();
  }
  $header = array(
    'title' => array(
      'data' => t('Title'),
      'specifier' => 'filename',
      'type' => 'property',
      'field' => 'f.filename',
    ),
    'type' => array(
      'data' => t('Type'),
      'specifier' => 'filemime',
      'type' => 'property',
      'field' => 'f.filemime',
    ),
    'folder' => array(
      'data' => t('Folder'),
      'specifier' => array(
        'field' => 'field_folder',
        'column' => 'tid',
      ),
      'type' => 'field',
    ),
    'size' => array(
      'data' => t('Size'),
      'specifier' => 'filesize',
      'type' => 'property',
      'field' => 'f.filesize',
    ),
    'author' => array(
      'data' => t('Author'),
      'specifier' => 'uid',
      'type' => 'property',
      'field' => 'u.name',
    ),
    'timestamp' => array(
      'data' => t('Updated'),
      'specifier' => 'timestamp',
      'type' => 'property',
      'sort' => 'desc',
      'field' => 'f.timestamp',
    ),
    'operations' => array(
      'data' => t('Operations'),
    ),
  );
  $destination = drupal_get_destination();
  $options = array();
  $media_entities = media_browser_plus_load_multiple(array(
    'conditions' => array(),
    'header' => $header,
  ));
  // Gather the data from each media object.
  foreach ($media_entities->results as $media) {
    // Load folder name.
    $folder = '';
    if (isset($media->field_folder[LANGUAGE_NONE][0]['tid'])
    && ($term = taxonomy_term_load($media->field_folder[LANGUAGE_NONE][0]['tid']))) {
      $folder = $term->name;
    }

    // Load user object if media author is not the current user.
    if ($media->uid != $user->uid) {
      $media_user = user_load($media->uid);
    }
    // Media author is the same as current user no need to load the user.
    else{
      $media_user = $user;
    }

    $options[$media->fid] = array(
      'title' => theme('media_link', array('file' => $media)),
      'folder' => $folder,
      'type' => check_plain($media->filemime),
      'size' => format_size($media->filesize),
      'author' => theme('username', array('account' => $media_user)),
      'timestamp' => format_date($media->timestamp, 'short'),
    );
    $options[$media->fid]['operations'] = l(t('edit'), 'file/' . $media->fid . '/edit', array('query' => $destination));
  }
  // Grab old options.
    $ops = $form['options']['operation']['#options'];

  // Redo the options form.
  unset($form['options']);
  $form['options'] = array(
    '#type' => 'fieldset',
    '#title' => t('Operations'),
    '#weight' => -10,
    'operations' => array(
      '#type' => 'container',
      '#prefix' => '<div class="container-inline">',
      '#suffix' => '</div>',
      'operation' => array(
        '#type' => 'select',
        '#options' => $ops,
        '#default_value' => 'delete',
      ),
      'submit' => array(
        '#type' => 'submit',
        '#value' => t('Submit'),
        '#submit' => $form['#submit'],
      ),
    ),
    'folder' => array(
      '#type' => 'container',
      'selected_folder' => array(
        '#type' => 'select',
        '#title' => t('Media Folder*'),
        '#required' => FALSE,
        '#options' => _media_browser_plus_folder_list(),
      ),
      'submit' => array(
        '#type' => 'container',
        '#prefix' => '<div class="container-inline">',
        '#suffix' => '</div>',
        'submit' => array(
          '#type' => 'submit',
          '#value' => t('Change Folder'),
          '#submit' => array('media_browser_plus_media_admin_folder_change_submit'),
        ),
        'notice' => array(
          '#type' => 'item',
          '#markup' => t('*: change will be applied to ALL selected media files'),
        ),
      ),
    ),
  );
  $form['files']['#header'] = $header;
  $form['files']['#options'] = $options;

  // Reset pager.
  $form['pager'] = array('#markup' => _media_browser_plus_pager($media_entities->page, $media_entities->pages));
  

  return $form;
}

Fixed


/**
 * Alter the media list display.
 *
 */
function _media_browser_plus_media_admin_list_alter(&$form, &$form_state) {
  global $user;

  if (array_key_exists('operation', $form_state['input']) == TRUE && $form_state['input']['operation'] == 'delete') {} else {

  // Check access rights.
  if (!media_browser_plus_access('media list view')) {
    drupal_access_denied();
    return array();
  }
  $header = array(
    'title' => array(
      'data' => t('Title'),
      'specifier' => 'filename',
      'type' => 'property',
      'field' => 'f.filename',
    ),
    'type' => array(
      'data' => t('Type'),
      'specifier' => 'filemime',
      'type' => 'property',
      'field' => 'f.filemime',
    ),
    'folder' => array(
      'data' => t('Folder'),
      'specifier' => array(
        'field' => 'field_folder',
        'column' => 'tid',
      ),
      'type' => 'field',
    ),
    'size' => array(
      'data' => t('Size'),
      'specifier' => 'filesize',
      'type' => 'property',
      'field' => 'f.filesize',
    ),
    'author' => array(
      'data' => t('Author'),
      'specifier' => 'uid',
      'type' => 'property',
      'field' => 'u.name',
    ),
    'timestamp' => array(
      'data' => t('Updated'),
      'specifier' => 'timestamp',
      'type' => 'property',
      'sort' => 'desc',
      'field' => 'f.timestamp',
    ),
    'operations' => array(
      'data' => t('Operations'),
    ),
  );
  $destination = drupal_get_destination();
  $options = array();
  $media_entities = media_browser_plus_load_multiple(array(
    'conditions' => array(),
    'header' => $header,
  ));
  // Gather the data from each media object.
  foreach ($media_entities->results as $media) {
    // Load folder name.
    $folder = '';
    if (isset($media->field_folder[LANGUAGE_NONE][0]['tid'])
    && ($term = taxonomy_term_load($media->field_folder[LANGUAGE_NONE][0]['tid']))) {
      $folder = $term->name;
    }

    // Load user object if media author is not the current user.
    if ($media->uid != $user->uid) {
      $media_user = user_load($media->uid);
    }
    // Media author is the same as current user no need to load the user.
    else{
      $media_user = $user;
    }

    $options[$media->fid] = array(
      'title' => theme('media_link', array('file' => $media)),
      'folder' => $folder,
      'type' => check_plain($media->filemime),
      'size' => format_size($media->filesize),
      'author' => theme('username', array('account' => $media_user)),
      'timestamp' => format_date($media->timestamp, 'short'),
    );
    $options[$media->fid]['operations'] = l(t('edit'), 'file/' . $media->fid . '/edit', array('query' => $destination));
  }
  // Grab old options.
  if (array_key_exists('options', $form)) {
    $ops = $form['options']['operation']['#options'];
  } else {
    $ops = array();
  }
  // Redo the options form.
  unset($form['options']);
  $form['options'] = array(
    '#type' => 'fieldset',
    '#title' => t('Operations'),
    '#weight' => -10,
    'operations' => array(
      '#type' => 'container',
      '#prefix' => '<div class="container-inline">',
      '#suffix' => '</div>',
      'operation' => array(
        '#type' => 'select',
        '#options' => $ops,
        '#default_value' => 'delete',
      ),
      'submit' => array(
        '#type' => 'submit',
        '#value' => t('Submit'),
        '#submit' => $form['#submit'],
      ),
    ),
    'folder' => array(
      '#type' => 'container',
      'selected_folder' => array(
        '#type' => 'select',
        '#title' => t('Media Folder*'),
        '#required' => FALSE,
        '#options' => _media_browser_plus_folder_list(),
      ),
      'submit' => array(
        '#type' => 'container',
        '#prefix' => '<div class="container-inline">',
        '#suffix' => '</div>',
        'submit' => array(
          '#type' => 'submit',
          '#value' => t('Change Folder'),
          '#submit' => array('media_browser_plus_media_admin_folder_change_submit'),
        ),
        'notice' => array(
          '#type' => 'item',
          '#markup' => t('*: change will be applied to ALL selected media files'),
        ),
      ),
    ),
  );
  $form['files']['#header'] = $header;
  $form['files']['#options'] = $options;

  // Reset pager.
  $form['pager'] = array('#markup' => _media_browser_plus_pager($media_entities->page, $media_entities->pages));
  
  }

  return $form;
}

This change fixes the two sets of bugs entirely!

The second set of errors were triggered as a direct result of the form elements being included in the post-back to the server. The script didn't know what to do with the extra form elements.

By suppressing the form elements during the "Deletion" operation the entity_file module can do it's job.

Jackinloadup’s picture

Here is a patch of the suggested solution in #12 rolled into #5.

The result is as desired from my quick testing.

das-peter’s picture

Status: Needs review » Fixed

Oh nice! I already fixed the deletion partially (the easy part in the thumbnail view ;) ) but then I luckily found this patch.
Applied, extended and committed: http://drupalcode.org/project/media_browser_plus.git/commit/fa2daf73599c...

Status: Fixed » Closed (fixed)

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

drdam’s picture

Hi,

I using :
File entity : 7.x-2.0-unstable7 - 2012-11-18
Media : 7.x-2.0-unstable7 - 2012-11-18
Media Browser Plus for Media 2.x : 7.x-2.x-dev - 2012-10-30

When I want to dellete multiple files, I had this error message :

Notice : Undefined index: file_multiple_delete_form dans drupal_retrieve_form() (ligne 763 dans /durpal_core/includes/form.inc).
Warning : call_user_func_array() expects parameter 1 to be a valid callback, function 'file_multiple_delete_form' not found or invalid function name dans drupal_retrieve_form() (ligne 798 dans /durpal_core/includes/form.inc).

In fact , the bug are produce by Media Browser Plus in media_browser_plus.pages.inc on line 54-55

module_load_include('inc', 'file_entity', 'file_entity.pages');
$form = drupal_get_form('file_multiple_delete_form', $files);

Or in file_entity.pages.inc line 450, the function is "file_entity_multiple_delete_form".

After remlacing it in media_browser_plus.pages.inc, the bug is solved