Call to a member function isDisplayed() on null error

Since we may have a new maintainer or maintainers soon, I will just add a reminder that this should be committed. I don't see any way that I can rtbc this or I would.

There are a few loose ends that should be cleaned up once this project is being maintained again.

Just found where to rtbc this under issue metadata. So I did. I have been running this modified code on both test and production systems since it was first proposed.

I suddenly got this error.

But the reason had nothing to do with the plugin itself.

The formatter for a taxonomy term in Manage Display had taken Juicebox Gallery by default. I hadn't noticed it.

Changing the formatter to Label obviously solved the problem.

Just noticed that this request is for 8.x-3-dev. This won't automatically update the alpha2 release which is what Drupal8 and 9 users should be using now. Just checked the 4.0 code and the bug is in there too. Which is natural since that derives from 8.x-3.x-dev.

Maybe Luke can review and apply the patch to all 3 if appropriate. I've been running a manually applied version of the patch on both my older (now defunct) 8.x system and now on my 10.0.7 test system.

I have a feeling that there are some unseen factors at play here.

We might have a better time at implementing a method in the formatter that prevents this from happening in the first place. This will straight up remove the formatter from site builders' options so that this condition can't exist in the first place for folks to even see :-).

  /**
   * {@inheritdoc}
   *
   * Ensure that only media types sourced through images are referenceable.
   */
  public static function isApplicable(FieldDefinitionInterface $field_definition) {
    $applicable = parent::isApplicable($field_definition);
    if ($applicable) {
      if ($field_definition->getSetting('target_type') === 'media') {
        $handler_settings = $field_definition->getSetting('handler_settings');

        // If no target bundles are set, assume all are allowed.
        $allowed_bundles = $handler_settings['target_bundles'] ?? array_keys(\Drupal::service('entity_type.bundle.info')->getBundleInfo('media'));

        // Filter out all valid bundles.
        $invalid_bundles = array_filter($allowed_bundles, static function($allowed_bundle) {
          return MediaType::load($allowed_bundle)->getSource()->getPluginId() !== 'image';
        });

        // Only apply to this field if there are no invalid bundles!
        $applicable = empty($invalid_bundles);
      }
    }

    return $applicable;
  }

Furthermore, we should probably consider hardening the implementation a bit. field_media_image is site builder controlled -- not a guarantee! The Media API has us covered though.

      if ($item->getPluginId() === 'field_item:entity_reference') {
        /** @var \Drupal\media\MediaInterface $media_entity */
        $media_entity = $item->entity;

        // Load the media type of the entity.
        $media_entity_type = MediaType::load($media_entity->bundle());
        
        // Dynamically figure out what the source field is configured to be.
        $source_field = $media_entity->getSource()->getSourceFieldDefinition($media_entity_type);

        $item = $media_entity->get($source_field->getName())->first();
      }

should be a bit more robust.

Acceptance test criteria should probably be that the Juicebox formatter would only appear as an option for site builders when...

1. A field target is 'image'
2. A field target is 'file'
3. A field target is 'entity_reference' AND the entity reference is to 'media' AND the bundles that content editors are allowed to choose all use the 'image' media source type

Does that make sense?

I'm setting this back to NW on #9 - the UX with the patch as presented means that sites with invalid configuration wouldn't crash, but would probably instead render empty div elements -- which might go un-noticed for quite a while.

I think that preventing invalid configuration is worth the additional complexity in this case. It's hard to tell what other bugs invalid configuration could incur -- perhaps even security issues in some cases!

This new test should prove that there's a problem...and form a base for new Media integrations tests.

This should make it impossible to configure new sites incorrectly, however we should also add a warning for site owners that have incorrect configurations present in active storage.

Added a requirements check and attached a draft change record for site builders: https://www.drupal.org/node/3403257 →

Resolve coding standards violations that were in scope for this changeset.

Must be getting tired. Here's the correct patch...heh. #18 had an improperly named test module (from another issue in Juicebox :-) ).

...and one more change to rename the test case to something more appropriate.

Reroll.

Committed and pushed to 4.0.x - thanks all.

Luke.Leber → closed merge request !8

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