hook_field_prepare_view() is passed empty field values for EntityNG Entities

Confirming a fatal is thrown, hence the theme blows up

Loss of theme/page output is critical.

$item['path'] doesn't exist so far as I can see.
$item['options'] needs to be something else we get unsupported operand types.

What happened to hook_field_prepare_view()?
http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/link/li...

Why is it not called for blocks?

Sun pointed out that link_field_prepare_view not running for custom block.

So turns out hook_field_prepare_view() isn't passed the correct field values if the entity is an NG entity.
We need to invoke the BC.

+++ b/core/modules/field/field.attach.incundefined
@@ -524,7 +524,11 @@ function _field_invoke_multiple($op, $entity_type, $entities, &$a = NULL, &$b =
+          // Enable BC if necessary.
+          $entity = $entity->getBCEntity();
           $grouped_items[$field_id][$langcode][$id] = isset($entity->{$field_name}[$langcode]) ? $entity->{$field_name}[$langcode] : array();
+          // Reset back to original.
+          $entity = $entity->getOriginalEntity();

@@ -565,6 +568,8 @@ function _field_invoke_multiple($op, $entity_type, $entities, &$a = NULL, &$b =
+          $entity = $entity->getBCEntity();
           $entity->{$field_name}[$langcode] = $grouped_items[$field_id][$langcode][$id];

I'd think you'd have to set it back again in the second hunk as well? Not sure if that's the cause of the fails, I didn't try it.

Resetting back to original entity as per #11

New title

Wrong view mode.
Also, extra $language variable isn't used.

At the bottom of #1818560-33: Convert taxonomy entities to the new Entity Field API (below the last horizontal rule, "Finally: manual testing…"), I explained in detail what seems to be the same problem. Berdir pointed me to this issue in #34 of that issue.
I applied the patch in #16 to check if it'd fix things. Unfortunately, it does not, it causes the site to break entirely:
InvalidArgumentException: Property uuid is unknown. in Drupal\image\Type\ImageItem->setValue() (line 101 of <drupalroot>/core/modules/image/lib/Drupal/image/Type/ImageItem.php).

(This is on the taxonomy term page of a taxonomy term that has an image field associated with it.)

So, AFAICT, this patch has the potential to break Drupal, hence marking as NW.

Finally, AFAICT this is not isolated to "link" fields, since image fields are what's failing for me. If I'm conflating two different issues, then my apologies and feel free to go back to NR.

See also the discussion in #1839068: Implement the new entity field API for the file field type where we already discovered and discussed some of these issues but then forgot about it.

#16: link-formatter-1957748.16.patch queued for re-testing.

Sorry that it took me so long to have a look at this :)

field_attach_*() function never should be called with an EntityNG object, that simply doesn't work. So the fault is in EntityRenderController. The existing link field test already fail when ported to use EntityTest (which is NG) instead of TestEntity (which is not), so I just copied over my fix from #1822000: Remove Drupal\field_test\Plugin\Entity\Type\TestEntity in favor of EntityTest, where I'm working on this.

Given that, not sure if we need additional test coverage for this. that issue will provide test coverage as soon as we switch to NG and then this will be mostly a test duplicate. But I kept it for now. Can also try to get over just the link field tests from that issue not sure if they can be isolated easily.

Also fixed ImageItem to work like FieldItemBase and accept all additional things values, at least for now until we have converted everything, then we can switch to use ->entity for this.

Ok, those tests actually call field_attach_*() directly, must have been others that failed. I do wonder if those tests should be converted to use EntityTest instead of a real entity and live in link.module or field.module?

Works perfectly.

Unassigning @larowlan :)

+++ b/core/lib/Drupal/Core/Entity/EntityRenderController.phpundefined
@@ -28,7 +28,11 @@ public function __construct($entity_type) {
   public function buildContent(array $entities, array $displays, $view_mode, $langcode = NULL) {
-    field_attach_prepare_view($this->entityType, $entities, $displays, $langcode);
+    $bc_entities = array();
+    foreach ($entities as $entity) {
+      $bc_entities[$entity->id()] = $entity->getBCEntity();
+    }
+    field_attach_prepare_view($this->entityType, $bc_entities, $displays, $langcode);
     module_invoke_all('entity_prepare_view', $this->entityType, $entities, $displays, $view_mode);

I think we could do with a comment to explain what's going on here...

Actually, I think that what was going on there is totally unnecessary. Both field_attach_prepare_view() and field_attach_view() have the swtich to BC entities incorporated, the problem was that in field_attach_prepare_view() it was called a bit late.

Oh, didn't know that.

It's a bit inconsistent with all other field_attach_*() functions right now, which except a BC entity. Maybe we should change all these others in a follow-up as well? That would simplify #1822000: Remove Drupal\field_test\Plugin\Entity\Type\TestEntity in favor of EntityTest a lot.

Sure, anything that makes that patch easier gets a huge ++ from me :)

Good fix and comes with tests -> RTBC.

Committed 0261175 and pushed to 8.x. Thanks!

Also, this doesn't have anything to do with link.module, it's more like.. Field API meets EntityNG, unpredictable things are bound to happen :)

Thanks, Alex!

#34: Indeed, that's what I'd been thinking too :)

Thanks!

Opened #1974474: Make sure within field_attach_*() that we are working with a BC entity