Problem/Motivation

Steps to reproduce

Given a simple node type
With a field for attaching media too
And you Attach two media items to that field on a node
And you go delete the first media item in /admin/content/media
When you Edit the node
You get the following error:

Error: Call to a member function id() on null in insert_media_insert_variables() (line 135 of modules/contrib/insert/modules/insert_media/insert_media.module).

Proposed resolution

The insert_media.module function insert_media_insert_variables() makes the assumption that 0 is always available. It isn't. If you delete 0 elsewhere, only 1 will be available, but there are no sanity checks so the error flies!

Remaining tasks

User interface changes

API changes

Update insert_media_insert_variables() to iterate over all keys until it finds a numeric key and use that. If it doesn't fund any, well it does nothing and that's okay somehow too.

Data model changes

CommentFileSizeAuthor
#3 drupal-insert-3519353-1-fix-assumptive-presence-of-first-media-item.patch996 bytesjnicola
#2 drupal-insert-3519353-1-fix-assumptive-presence-of-first-media-item.patch975 bytesjnicola

Comments

jnicola created an issue. See original summary.

jnicola’s picture

jnicola commented
StatusFileSize
new drupal-insert-3519353-1-fix-assumptive-presence-of-first-media-item.patch975 bytes

Attached a patch that works for us. I recognize the better way to do it is a commit in a fork but... eh I couldn't be bothered and since nobody else seems to be having this problem it likely doesn't matter.

Solution though was simple. Instead of assuming the key of 0 exists, just iterative over keys until you find the first numerical key, and run with that if you get it. Further improvements are likely possible, but this works and doesn't feel hacky to me.

jnicola’s picture

jnicola commented
StatusFileSize
new drupal-insert-3519353-1-fix-assumptive-presence-of-first-media-item.patch996 bytes

Uploading patch with slight tweak after code review. Original code returned based on first item value, so now we just return on the first found media item versus assuming the first item is 0.

snater’s picture

snater commented

Thank you for the patch! Definitely an improvement that should be merged in. Just one note: Shouldn't the check whether actually processing inserting media be kept to prevent potential interference with other hooks, and abort processing the hook early if not relevant for the current operation? Just adding an early return:

function insert_media_insert_variables($insertType, array &$element, $styleName, &$vars) {
  if ($insertType !== INSERT_TYPE_MEDIA) {
    return;
  }

  foreach ($element['selection'] as $key => $val) {
    if (is_int($key)) {
      $vars['media_entity'] = $element['selection'][$key]['rendered_entity']['#media'];
      $mid = !is_null($vars['media_entity']) ? $vars['media_entity']->id() : 0;
      $element['#name'] = 'media_ ' . $mid;
      return;
    }
  }
}