Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Title: direct calls to [comment|node]_build() do not trigger f_a_prepare_view() » direct calls to node_view() do not trigger f_a_prepare_view()
Status: Active » Needs review
FileSize
5.79 KB

So the issue is that
- f_a_prepare_view() lets formatters load additional data (e.g. term object for a 'term field' tid value) so it's a 'multiple objects' hook, to allow multiple objects queries.
- f_a_prepare_view() needs to run before f_a_view()
- node_view_multiple($nodes) currently calls f_a_prepare_view($nodes), then loops on node_view($node) / node_build_content($node)
- node_build_content($node) doesn't call f_a_prepare_view() so that it doesn't run twice, and directly calls f_a_view().
So direct calls to node_view() - such as when previewing a comment on a node - skip the prepare_view step and formatters might not find the data they expect.
Current HEAD has code to prevent warnings in the taxo field formatters.

Note that comment.module uses the same comment_view_multiple() / comment_view() / comment_build_content() workflow, but comment_build_content() does call f_a_prepare_view(). So f_a_prepare_view() runs twice there...

Attached patch is quite similar to what #636992: Entity loading needs protection from infinite recursion does for entity_prepare_view()
- f_a_prepare_view() sets a $object->_field_view_prepared flag, and doesn't act on incoming objects with the flag on.
The '_' in front of the name is to prevent potential clashes with a UI-created field named 'view_prepared'
- The same object, or a subset of its fields, might be re-displayed using different formatters later in the request (e.g in a side block). In this case, the prepare_view steps need to run again. So just before it returns, f_a_view() unsets the flag that was set in f_a_prepare_view().
- removes the temporary fixes around taxonomy formatters.
- enhances the PHPdoc for f_a_prepare_view()

catch’s picture

This looks great, I found one typo:

+++ modules/field/field.attach.inc	31 Dec 2009 02:12:34 -0000
@@ -1111,15 +1111,41 @@ function field_attach_query_revisions($f
+ * therefore accepts an aray of objects to allow query optimisation when
+ * displaying lists of objects.

s/aray/array

Also my American English spell checker wants optimization but I try to slip random UK English spellings into core whenever I'm able to so I won't say anything about that.

Otherwise RTBC.

This review is powered by Dreditor.

yched’s picture

Thx ! Typo fixed.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

RTBC now based on catch's review

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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