As per current dev, drupal_alter() is called for media_wysiwyg_allowed_view_modes and media_wysiwyg_wysiwyg_alAlowed_view_modes in media_wysiwyg_get_file_view_mode_options() on media_wysiwyg.module. But $view_modes available is just one dimensional array of key label pairs, where hook_media_wysiwyg_allowed_view_modes_alter() and hook_media_wysiwyg_wysiwyg_allowed_view_modes_alter() are supposed to recieve multi dimensional array of view mode details.

In my case, it is making problems in function panopoly_images_media_wysiwyg_wysiwyg_allowed_view_modes_alter() of panopoly_images module:

/**
 * Implements hook_media_wysiwyg_wysiwyg_allowed_view_modes_alter().
 */
function panopoly_images_media_wysiwyg_wysiwyg_allowed_view_modes_alter(&$options, $context) {
  if ($context->type == 'image') {
    // Relabel some options
    $labels = array(
      'default' => t('Original Size'),
      'teaser' => t('Quarter Size'),
      'preview' => t('Thumbnail'),
    );
    foreach ($labels as $name => $label) {
      if (isset($options[$name])) {
        $options[$name]['label'] = $label;
      }
    }
  }
}

In above function it expected $options as multi dimensional array and trying to modify label property of view mode item.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

junaidpv created an issue. See original summary.

junaidpv’s picture

Here is a dirty patch to fix it temporarily. It changes signature of media_wysiwyg_get_file_type_view_mode_options() to receive extra file object and moving drupal_alter() calls.

junaidpv’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: incorrect_alter_hook_call-2566509-2.patch, failed testing.

Dave Reid’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
1.32 KB

Hrm, I hadn't known anyone that was using the alter hook, sorry. It was actually incorrectly named, so I think we should just deprecate it entirely. You'll find that you'll want to do basically the same logic with the new hook, just with simpler data:

/**
 * Implements hook_media_wysiwyg_allowed_view_modes_alter().
 */
function panopoly_images_media_wysiwyg_allowed_view_modes_alter(&$options, $context) {
  if ($context->type == 'image') {
    // Relabel some options
    $labels = array(
      'default' => t('Original Size'),
      'teaser' => t('Quarter Size'),
      'preview' => t('Thumbnail'),
    );
    foreach ($labels as $name => $label) {
      if (isset($options[$name])) {
        $options[$name] = $label;
      }
    }
  }
}
junaidpv’s picture

@Dave Reid, what about that hook is getting called with two different type of data?

In media_wysiwyg_get_wysiwyg_allowed_view_modes() it is called with more detailed view mode info and in media_wysiwyg_get_file_view_mode_options() it is called with just key/value pairs.

Dave Reid’s picture

I believe it was introduced in error, in an attempt to fix the hook name that had '_wysiwyg' duplicated in it twice. The preferred hook is the one with simpler data, as invoked from media_wysiwyg_get_file_view_mode_options(). To support both older and newer versions of media_wysiwyg, I would recommend having both hooks in your module.

joseph.olstad’s picture

Status: Needs review » Closed (outdated)

the second hunk still applies, however hesitant to remove the hook unless some recent testing on newer versions of media