View mode no longer can be changed

Subscribe, I'd love to be able to rely on that in http://drupal.org/project/userpoints_nodeaccess which switches to a different view mode if you don't have access.

Sorry, but can you clarify what kind of edge-case scenario makes this issue major? If you want a particular $view_mode, then call node_view() with it.

I'm fixing the issue tags. But even if this may have been possible in CCK/D6, I not really sure whether it makes sense to support this in D7.

Some bigger use cases:

• Instead of desktop site, you are now serving to a mobile phone - and not only show nodes as a page, but also in lists etc. (mobile tools for instance is completely broken because of this)
• Using any sort of context (either through context module or CTools') to change a view mode on any sort of condition (where the first one is one of those options in way) A couple of modules provided this for context and are now completely broken.

Also, since view modes are now available on all entities, let's add them everywhere - patch adds them as well on comments, terms and users.

Agree that this is major for the exact reason Swentel lists first-- but for testing, where is it possible to change the view mode? With the patch (applies as is to Drupal 7, will need to re-roll for 8 post /core) changing the view mode in preprocess is apparently still too late.

You can set $entity->view_mode in http://api.drupal.org/api/drupal/modules--system--system.api.php/functio....

The view mode that is passed into node_build_content() is from the second parameter of node_view() which gets it from the second parameter of node_view_multiple($nodes, $view_mode = 'teaser', ...), not from the node object, and it is in node_view() that the view mode is assigned to the build array.

Therefore in this call stack, which is used when showing a node as a full page (and also, for that matter, when showing a node in a view rendered through Display Suite) changing the view mode in the node entity object with hook_entity_prepare_view() has no effect (Drupal 7).

For nodes, node_view() is where it matters what the view mode is. It calls node_build_content() in which hook_entity_prepare_view() is invoked after field_attach_prepare_view() (which receives the view mode as an argument). (Also, hook_entity_prepare_view() is not invoked for full nodes shown on their own page in node_build_content(), which is where this patch adds the $node->view_mode = &$view_mode;, line, because node_show(), which hardcodes the full display mode, uses node_view_multiple() marks the nodes as already prepared.)

Regardless, changes to the view mode in implementations of hook_entity_prepare_view() are not reflected later in node_view() when the $build array is populated and the view mode assigned to #view_mode.

Using hook_node_view() alter to change just what is assigned the renderable array with #view_mode is effective apparently but does not change any of these earlier uses of view mode, including when doing the attaching fields preparation.

Alright I've just written some contrib code that checks $entity->view_mode in Drupal 7. Doesn't work yet, of course, but it's in there now, so now everyone has to go along with it. :D

I also noticed node_show() assumes a 'full' view mode. Can't we check for $node->view_mode first, and then go on? This would allow us to change build mode through hook_node_load() and might save the performance from loading the wrong build mode and its fields — for what it's worth.

Attached is a patch that implements that. I believe it adds to swentel's patch.

barraponto: I think you'll want to include the changes from the previous patch in yours, then set the issue to 'Needs review' to trigger the testbot.

There it is, merged with swentel's patch from comment #6.

+1 for getting this in. I'm looking at needing to do a domain access based view mode switch.

Note that this is possible in D7 by overriding the node/% menu callback and replicating everything bar the view mode changes, but that is just plain nasty!

Following up on mlncn comments, how do you use this feature???

From testing using a number of different entity / field hooks had no effects with the supplied change. I didn't try any node hooks as I'm trying to get this working at the entity level.

@Alan D.: What's stoppin this patch from getting in at the moment is a lack of an automated test that duplicates the bug. The test should fail without the patch here and pass with it. Thanks!

@xjm I'm not saying that there is an issue, I'm saying that I'm not sure how a developer will use it. We need to flick view modes per domain using domain access, and without overriding either the page callback or rendered #build, I can not see how to implement this.

Wondering exactly the same.

Shame this didn't get it in for 7.12...

The problem is that we need to find an early spot to alter the view mode, because as mlncn says, hook_entity_prepare_view simply wont' work. I think this rather calls for an extra alter hook, I'll see if I can experiment with this today.

I agree with #20, there should probably be a new hook giving modules the opportunity to alter $view_mode for any entity before loading.

For those like me who are in a need for this mechanism, I've setup a sandbox project which exposes the approach Berdir uses in userpoint_nodeaccess by means of a hook_modeswitch_node_view_mode_alter(&$view_mode, $original_view_mode, $node). Keep in mind that this approach effectively results in loading nodes twice.

Domain View Modes does this too, by completely rebuilding the rendered node late in the view process within hook_entity_view_alter().

#15 I agree.

chx - thanks for pushing for this. I really hope to see this backported to D7. The idea of view modes without being able to switch when viewing a node via node/123 is nonsense. It's great for Views, embedded nodes, blocks, etc., but change view mode depending on context or role is quite powerful.

I really look forward to seeing this backported to D7.

Looking into a test.

Cool, as least the tests will actually tell us how to use this!!

Alright, so as far as I can tell #14 does not actually work. chx suggested instead adding a hook_entity_view_mode_alter().

The reason that it doesn't work is that the only viable hook for this is hook_entity_prepare_view(). And that hook is already called in node_view_multiple() where there is no $node->view_mode.

The attached patch adds it early enough to nodes to check if that would theoretically work, as less sense as it makes (see comment).

Will try to create a patch using a new hook next.

Got distracted, attached is the version with the new hook. Completely untested but should work :) There is even the start of documentation for it ;)

There is one limitation of this approach, though:
This works for both entity_view() and entity_view_multiple(). However, in the case of entity_view_multiple(), the (field/entity)_prepare_view hooks run before the view_mode can be changed. entity does not receive the view mode, so that's not relevant, but field does so a module could possibly do something based on the view mode, then that changes and things are messed up.

I can only see one proper solution for that:
We change view/view_multiple() to work like load does. view() is nothing more but a wrapper for view_multiple() then we need exactly one place where we call the prepare_view hooks and can throw away the double-invocation protection and this also works reliably in all cases. Will hopefully happen when we introduce entity_view(_multiple)() in core...

I can work with this. :)

Thanks Berdir. That hook was sorely needed in my case where we are using Mobile Tools to display content differently on various devices, in conjunction with Entity View Modes. I can now tell a user or node to load in a custom view mode on its detail page other than 'full' which would affect the desktop site.

That is awesome. Does this have a chance of being backported into 7?

@csdco: It is tagged "needs backport to D7," so yes, we will try to make it backportable.

Works great, thanks Berdir

Although the patch provided in #28 makes a change on module/entity/entity.api.php which doesn't exist in the core.
So I put it in module/system/system.api.php and it does the trick!

Cheers

Same, #28 works well but what is that path? I also used system.api.php.

This will need a reroll, for D8 as we entity is psr-0 now

I'll try and re-roll this.

Hi again,

I notice this patch is not working with context module when I choose "node type" condition.

Do you have any idea why this happen?

Thanks in advance

Problem came from context module in context.core.inc

/**
 * Implementation of hook_node_view().
 */
function context_node_view($node, $view_mode) {
  if ($view_mode === 'full') {
    $object = menu_get_object();
    if (isset($object->nid) && $object->nid === $node->nid) {
      context_node_condition($node, 'view');
    }
  }
}

I just added this hook to my own module and it works fine

/**
 * Implementation of hook_node_view().
 */
function MYMODULE_node_view($node, $view_mode) {
  if ($view_mode === 'myviewmode') {
    $object = menu_get_object();
    if (isset($object->nid) && $object->nid === $node->nid) {
      context_node_condition($node, 'view');
    }
  }
}

Please don't change the issue title.

@lahode The hook_node_view will never work, see the patch

I re-rolled the patch from #28. Note that only the parameters changed. The interdiff is kind of irregular since I didn't actually add anything to the patch but shows the changes.

This still needs a test.

Let the bot at it.

Attached is a test for the node view mode change as well as the original patch.

Yay, tests!

Some coding style issues and a suggestion to simplify the test code below. This currently only tests nodes, but that is afaik the only entity type in core that does have different view modes and we don't have entity_view() yet, so we can't do a generic test. So IMHO, just testing nodes is ok, maybe add a @todo that we should try to generalize the test case for all entities once we have entity_view().

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeEntityViewModeAlterTest.phpundefined
@@ -0,0 +1,52 @@
+ * Test changing view modes for nodes.

TestS, I think.

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeEntityViewModeAlterTest.phpundefined
@@ -0,0 +1,52 @@
+    // Enable dummy module that implements hook_node_view.

should be "hook_node_view()".

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeEntityViewModeAlterTest.phpundefined
@@ -0,0 +1,52 @@
+
+    // Check that teaser mode is viewed.
+    $this->assertText('Extra data that should appear only in the teaser for the node.', "Teaser text present");
+    // Make sure body text is not present.
+    $this->assertNoText('Data that should appear only in the body for the node.', "Body text not present");

The assertion messages should use single quotes as well.

+++ b/core/modules/node/tests/modules/node_test/node_test.moduleundefined
@@ -9,6 +9,21 @@
+function node_test_menu() {
+  $items = array();
+  $items['node/test/view-modes/%node'] = array(
+    'title' => 'Test View Mode Alter',
+    'page callback' => 'node_test_view_mode_alter_page',
+    'page arguments' => array(3),
+    'access arguments' => array('access content'),
+    'type' => MENU_SUGGESTED_ITEM,
+  );
+  return $items;

Instead of using a custom menu callback and relying on arg() for the check (which will likely be removed in 8.x due to the request/response objects), I suggest you simply go to node/view/$nid and set a one-off variable, e.g. variable_set('node_test_view_mode', 'teaser') and then check that one in your alter function. Should save you quite a bit of code in the test module.

I'll work on this. :-)

Hmmm... patch that was marked "green" is failing here. Let's test again.

#43: view-modes-no-longer-changed-1154382-41.patch queued for re-testing.

Okay. I think I've fixed the test that was passing before so that it passes again. Just part of the code (lines before the added lines) has changed in current 8.x, so it wouldn't apply that part. I've looked it over and patched that part manually. Now for the tests which were always failing...

Hmmm... okay. I've made all the trivial changes to NodeEntityViewModeAlterTest.php, but I'm not familiar enough with how test sub-modules work to be sure I'd make the right changes to the node_test.module. So running tests, as it stands, still fails in the same way. I'll submit patches since now the main patch at least can be applied successfully and the test patch will at least have the "trivial" (coding standards) fixes, but either someone else should finish this or I could use some guidance.

Lets run those tests... At least the main patch should apply, but I think the test is still failing.

The test is supposed to fail without the new hook :)

The idea is that you first upload the test-only changes, so that it's visible that it does file and second the combined patch.

Simplified the test as suggested before, see interdiff.

+++ b/core/modules/entity/entity.api.phpundefined
@@ -434,3 +434,15 @@ function hook_entity_prepare_view($entities, $entity_type) {
\ No newline at end of file

...

4 days to next Drupal core point release.

#51: view-mode-alter-1154382-51.patch queued for re-testing.

Uhm, that was a rather dumb variable name clash ;)

Fixed, also fixed the ending newline.

Looks great! Still applies.

Committers, note that it adds a new file.

+++ b/core/modules/entity/entity.api.phpundefined
@@ -434,3 +434,15 @@ function hook_entity_prepare_view($entities, $entity_type) {
+/**
+ * Change the view mode of an entity that is being displayed.
+ *
+ * @param string $view_mode
+ *   The view_mode that is to be used to display the entity.
+ * @param array $context
+ *   An array with the keys entity_type, entity and langcode.
+ */
+function hook_entity_view_mode_alter(&$view_mode, array $context) {
+

We need to make sure that all hooks have an example with them. Other hooks in this file have examples, so this needs to conform.

+++ b/core/modules/node/node.moduleundefined
@@ -1246,6 +1246,14 @@ function node_build_content(Node $node, $view_mode = 'full', $langcode = NULL) {
+  drupal_alter('entity_view_mode', $view_mode, $context);
+
   // The 'view' hook can be implemented to overwrite the default function
   // to display nodes.

Just asking (sorry, very distracted here at #mwds). In every other entity, this line is directly above the call to field_attach_prepare_view(), but here it's not. Here, it fires a $node = node_invoke($node, 'view', $view_mode, $langcode); first. Is that cool?

Added an example.

The placement of the hook is correct, this is the node type view callback/pseudo-hook that is specific to nodes, no other entities have that and the hook needs to be called before that.

We could theoretically remove the entity_type from the context now, and when doing that, we're left with just two arguments, so we could pass them directly instead of using a temporary $context variable, but it might be easier to do that in a follow-up issue because that won't be backportable. Or we can change it and then backport the patch in #58, opinions?

Updated the patch as described in #59, feel free to RTBC the one you like ;)

We could theoretically remove the entity_type from the context now

Why! Without this it is impossible to determine what type of object that we are getting. #60 is definitely not ready to go, but I haven't reviewed #58 Sorry, I was thinking about D7 not D8 (still on my first morning coffee)

Have you look at the example code in the hook documentation? $entity is a classed object now, $entity->entityType() and you're done.

Edit: Double post ;)

For D8, I'd actually prefer to do none of both, but rather:

  $context = array(
    'langcode' => $langcode,
  );
  drupal_alter('entity_view_mode', $view_mode, $entity, $context);

That gives the full primary context to work with, and any additional context in the $context bag -- it's possible that we might want or need to add further data to $context later on.

That said, I wasn't exactly sure whether it wouldn't be better to pass the $entity first; i.e., so as to retain consistency with other entity API hooks?

Furthermore, I wondered whether there is room for a entity-type-specific alter hook like we do elsewhere? E.g.,

  drupal_alter(array($entity->entityType . '_view_mode', 'entity_view_mode'), $entity, $view_mode, $context);

We can't use drupal_alter() if we want $entity to be the first argument. Not sure if an entity-type specific hook is necessary, that means lot's of documenation and the additional condition isn't going to hurt IMHO, this isn't a hook like hook_node_*() that hundreds of are implementing.

Hm. Already posted that.

Ok, like this?

Now we have 3 possible implementations to chose from ;)

We really need entity_view(), then we don't have to duplicate all that code...

Ok, that doesn't work, context needs to be a variable.

Updated for the $modules dependencies thingy.

I vote for for the patch in comment 70.

In most cases one will only need the view mode and the entity. All tough the context currently only holds one value, this solution gives us to option to add additional values without braking any existing code.

As for me this is RTBC but I leave it as is to see if it gets some more agreement.

I agree.

Note: The alternatives to #70 (which I like as well, for the reasons outlined in the previous commit) are #60 and #58 (would require a re-roll to remove entity_type).

Double post.

Cross post.

Committed and pushed to 8.x. Thanks! Moving to 7.x.

Attaching a backport that goes back to the comment #58 approach because that's the only thing that works for 7.x.

Rerolled.

I'll reroll it again once #1067120: Missing hook_taxonomy_term_view() or hook_entity_view() when viewing a term is committed (fingers crossed).

I'm not supposed to RTBC a patch that I rerolled, right? Doh. Otherwise this looks good.

I think for rerolls you can, but looks good to me as well

Awesome, thanks folks for your patience while we got the threshold situation under control.

Committed and pushed to 7.x! Also added a CHANGELOG.txt entry for this.

Sorry, but I have to reopen this one as there are multiple issues with it:

• D8: The new test file was not committed.
• D7: As already outlined in #10 the alteration to the view mode performed in ENTITY_build_content() is not propagated to the caller functions (usually ENTITY_view()). This may cause an inconsistent behavior for modules relying on the #view_mode key set in the returned build array. For instance Entity view modes uses that key to add template suggestions for the main entity templates (e.g. node--article--my_awesome_view_mode.tpl.php). Currently it cannot work because as soon as you implement the alter hook and change the node view mode from 'full' to 'my_awesome_view_mode' you get fields rendered with the custom view mode but the wrong template (node--article--full.tpl.php).
• D7: Taxonomy term view mode alteration is in the wrong place: instead of being inside taxonomy_term_build_content() it is in taxonomy_term_view(), which is inconsistent with the rest of core entities. A module trying to implement the view mode alter hook for all entities may see an inconsistent behavior.

The D8 patch is just a forward-port of the test-fix for the D7 bug + the original test file which didn't get committed.

The D7 patch fixes the bug by ensuring that the view mode key is set in the $entity->content key, which is more or less what D8 does. That value overrides the outdated one that would be set in the ENTITY_view() functions. It also updates the test to cover this and moves the drupal_alter() for Taxonomy terms call in the proper place.

Attached also interdiffs/test only versions for reviewer's comfort :)

+1 for this. @plach: thanks for catching our slip-up.

Yeesh! Sorry about that. Committed / pushed the missing test (drupal-view_modes-1154382-83.patch) to D8.

Back down to D7 for review on those changes.

Reuploading D7 patches.

Drupal 7.16 was a security release only, so this issue is now scheduled for Drupal 7.17 instead.

Fixing tags accordingly.

Also adding the "release blocker" tag (at least for now), since it seems there was a regression... and at a minimum we should understand exactly what that means before the next bug fix release of Drupal 7.

Agreed with the release blocker tag, however the initial backported patch already fixed the regression, the only problem is that it's only a partial fix. #87 should complete it.

:(

I reviewed this pretty carefully and think it's ready to go. I don't really like marking it RTBC myself (since I'll probably be committing it too), but I think this followup is pretty straightforward, and it looks like @DamienMcKenna reviewed it a bit also. And I really want to get this committed so we can get Drupal 7.17 out the door.

But if anyone else wants to review this in the next day or two, that would be highly, highly appreciated :)

A couple comments though:

1. I think we should really write a Drupal 7 change notification for this (overall) issue, especially if we expect other entity modules to start invoking this hook? (Either that, or change the hook_entity_view_mode_alter() documentation to mention that not all entities actually invoke it - something like that.) Right now we're "advertising" this as something that will happen for all entities, but that's not really the case. I'm adding the "needs change notification" tag for now.
2. My only issue with the followup patch itself is that the [entity]_view() functions all try to add #view_mode too, which now seems like it's deprecated by the fact that we're adding it earlier in [entity]_build_content()? But there's no harm in trying twice, I suppose.

3. +    // Test that the correct build mode has been set.
   +    $build = node_view($node);
   +    $this->assertEqual($build['#view_mode'], 'teaser', 'The view mode has correctly been set to teaser.');
   

   Probably this should be forward-ported to Drupal 8 once we're done here?

4. So along the lines of the regression pointed out above, it seems like we also have a problem in node_view():

   function node_view($node, $view_mode = 'full', $langcode = NULL) {
   ....
     // Populate $node->content with a render() array.
     node_build_content($node, $view_mode, $langcode);
   ....
     // Add contextual links for this node, except when the node is already being
     // displayed on its own page. Modules may alter this behavior (for example,
     // to restrict contextual links to certain view modes) by implementing
     // hook_node_view_alter().
     if (!empty($node->nid) && !($view_mode == 'full' && node_is_page($node))) {
       $build['#contextual_links']['node'] = array('node', array($node->nid));
     }
   

   Specifically, the contextual links code is using the original view mode rather than the altered one.

   I think this is a sufficiently small problem that we don't have to fix it right now (it's 50% an existing issue anyway), although presumably the fix simply involves having it check $build['#view_mode'] == 'full' instead.

Committed the followup to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/6e344fd49cdaf55a21f1ff16...

(Reviews certainly still welcome, though, especially before Drupal 7.17 is released.)

Moving this back to Drupal 8 for the other followups.

I took a stab at a Drupal 7 change notification here also: http://drupal.org/node/1833086

Updated the change notice with an example of how to use the new hook and described differences in 8.x so that we don't need a separate change notice there.

Thanks David!

I quickly tested this on a site where I'm currently using the sandbox mentioned in #21. It works well - though I only alter view modes on nodes.

Thanks @Berdir and @znerol!

The additions to the change notification look good to me, although the fact that it's now tagged for both D7 and D8 seems to mean that it doesn't show up for someone skimming through either of these places:
http://drupal.org/list-changes/drupal?to_branch=7.x
http://drupal.org/list-changes/drupal?to_branch=8.x

Not sure what to do about that - maybe it would be better as two separate change notices after all?

@David: Looks like we have a bunch of those that contain both branches already:

"8.x, 7.x": http://drupal.org/list-changes/drupal?keywords_description=&to_branch=8....

"7.x, 8.x": http://drupal.org/list-changes/drupal?keywords_description=&to_branch=7....

So this is quite common but you did point out a valid problem with it.

Suggestion: Change the view filter configuration from an Equals to Contains instead of having to duplicate all those change records?

Should we open a new issue in Webmaster and close this one?

Here is an issue for that #1867356: Change branch exposed filter on http://drupal.org/list-changes/drupal to CONTAINS, even if we decide to separate these change notices, it's still not something specific to this issue or needs a new one.

The view/render code has been completely rewritten in 8.x and a render controller has been added, not sure if there are still necessary follow-ups?

I'm pretty much sure I have found a bug which renders this hook unusable (at least for node pages, node/%).

To simulate the problem (using a clean D7.17 installation):

1. We have the content type 'Article', with view modes 'default' and 'teaser'.
2. 'Article' has a term reference field, allowing to reference taxonomy terms within the vocabulary 'Tags'.
3. In the 'default' view mode, we decide to only show the fields 'Title' and 'Body'.
4. In the 'teaser' view mode, we decide to show the fields 'Title', 'Body' and 'Tags'.
5. In a custom module we implement hook_entity_view_mode_alter as explained in the
   change record:

   function mymodule_entity_view_mode_alter(&$view_mode, $context) {
     // Change the view mode to teaser for anonymous users viewing a node.
     if (($context['entity_type'] == 'node') && user_is_anonymous()) {
       $view_mode = 'teaser';
     }
   }
   

   Now, when you visit a particular node of type 'Article' as an anonymous user, the 'teaser' view mode will be used.

6. Log out & visit a node of type 'Article', which references one or more terms of vocabulary 'Tags'.
7. You get an error page, displaying the following messages:
   

   Notice: Undefined index: taxonomy_term in taxonomy_field_formatter_view() (line 1596 of C:\htdocs\d7\modules\taxonomy\taxonomy.module).

   EntityMalformedException: Missing bundle property on entity of type taxonomy_term. in entity_extract_ids() (line 7633 of C:\htdocs\d7\includes\common.inc).

Basically, field_default_prepare_view() (which invokes hook_field_formatter_prepare_view()) is being run for the wrong view mode (in the example above, 'full'). This means taxonomy_field_formatter_prepare_view() is not being executed, causing the 'Undefined index: taxonomy_term' notice, and eventually causing the EntityMalformedException.

In the example above, if you add the 'Tags' field to the 'default' view mode, you will see the error disappears and all goes well (because taxonomy_field_formatter_prepare_view() will have been executed - although for the wrong view mode, but it makes the 'taxonomy_term' value available for later use).

Altough I haven't read this whole issue through - I *think* adding

drupal_alter('entity_view_mode', $view_mode, $context);

before the node_view_multiple() call in node_show() would help us out here. This means hook_entity_view_mode_alter() will be invoked in 2 places, node_show() and node_build_content() - but I can't see much harm in this.
An extra benefit: a useless call to field_attach_prepare_view() wouldn't happen anymore either.
-
FYI: I used taxonomy as an example, but have also tested this with the node_reference module: exactly the same behavior.

@RaF: I cannot reproduce this. I use the following drush sequence:

drush dl
cd drupal-7.18
drush site-install --db-url=sqlite:/dev/shm/test.sqlite
drush dl devel
drush en devel_generate
drush generate-terms tags 10
drush generate-content --types=article 10
cd sites/all/modules
mkdir mymodule
cd mymodule

Then create mymodule.info and mymodule.module according to the code from #103

drush en mymodule
drush instaweb

1. Then open a browser and login
2. navigate to admin/structure/types/manage/article/display/teaser, hide tags, save
3. Logout
4. Goto node/1

At this stage the teaser is displayed properly no errors are shown and neither watchdog nor php error log indicate any problems.

Edit: Forgot site-install in drush transcript

@znerol

Your step #2 is wrong, it's the other way around. Navigate to admin/structure/types/manage/article/display and make sure tags are hidden. Navigate to admin/structure/types/manage/article/display/teaser and make sure tags are visible (formatter for example rendered taxonomy term / link).

@RaF: Thanks, I'm able to reproduce the issue now.

I think we need to better align hook_entity_view_mode_alter with field_attach_prepare_view and entity_prepare_view. The latter two functions are designed to take multiple nodes in order to enable bulk loading of field values. However the alter-hook operates on one entity only.

It is clear that the alter-hook needs to be called before any of the prepare-functions, that means it must be called on each node directly on top of node_view_multiple. After that the prepare-functions need to be executed on each set of nodes which have a common view_mode separately.

A patch against D7 demonstrating the idea is attached (only for node.module so far). However the solution looks more than hackish and I hope someone has a better idea.

Can you confirm that this no longer happens in 8.x as we moved the code around there?

If so, this issue should probably be moved back to 7.x and get fixed there as I still don't know if there are any 8.x follow-ups here at all.

Can you confirm that this no longer happens in 8.x as we moved the code around there?

I confirm that the problem does not exist in drupal 8. Actually EntityRenderController::viewMultiple() does more or less the thing I proposed in #106, i.e. batch load / build fields per distinct view_mode. I consider this the way to go then.

I'll put together a patch for D7 based on that approach.

• Introduce new function entity_view_mode_prepare in common.inc invoking hook_entity_view_mode_alter and returning an array with arrays of entities keyed by view mode.
• Make sure that entity_view_mode_prepare is invoked before any call to field_attach_prepare_view except in node_preview.

Fix stupid mistake in entity_view_mode_prepare

OK, test passed, comments very welcome. I'm especially unsure about my usage of key here.

$view_mode = key(entity_view_mode_prepare('node', array($node->nid => $node), $view_mode, $langcode));

It looks good to me znerol.
Berdir just pointed out this issue to me but I created another one with and easy step to follow to reproduce #1968348: (Change notice update) hook_field_formatter_prepare_view does not make use of hook_entity_view_mode_alter causing major errors.
We may continue this discussion there to clean and close this issue as we will need tests for your new function.

Closing this issue then, let's continue in the new bug report.

See http://drupal.org/node/1154382

Sorry, I didn't read this carefully enough - a patch was committed for 7.17 (#81), release notes at http://drupal.org/node/1833086

But it didn't work properly, which is why the followup issue http://drupal.org/node/1154382 was created.

Untagging. Please remove the "Needs change notification" tag when the change notice task is complete.