Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Darren Oh’s picture

Status: Needs review » Needs work

Patch doesn't apply to current code.

Darren Oh’s picture

Status: Needs work » Closed (duplicate)

I believe this was fixed in CVS commit 214936.

Anonymous’s picture

Status: Closed (duplicate) » Active

I'm still seeing this with the June 17th dev version.

igor.ro’s picture

I think we could not solve this problem while we are using view embed method to show view field.

muhleder’s picture

Status: Active » Needs work

It's possible to override this behaviour, so that empty viewfields do not display (like other cck fields).

This is the code I'm using in my template.php to override the theme, would be nice if this could make it into the module so we don't have to fix this. I'll put a patch together if there's interest in testing it.

<?php

/**
 * Return a themed view avoiding viewfield recursion.
 *
 * Override viewfield default rendering so empty views
 * return empty output. This allows cck to detect that
 * the  field is empty and not display the field.
 * 
 */
function mytheme_viewfield_formatter_default($element) {
  global $_viewfield_stack;
  // For safety's sake, we can only display 2 levels of viewfields.
  if (count($_viewfield_stack) <= 2) {
    list($view_name, $display) = explode('|', $element['#item']['vname'], 2);
    $view_args = _viewfield_get_view_args($element['#item']['token_enabled'], $element['#item']['vargs'], $element['#node']);
    $node = $element['#node'];
    // Need to prevent recursive views and node building, but don't need to do
    // it on new node previews.
    if ($node->nid) {
      _viewfield_nodestack_push($node->nid);
    }
    $view = views_get_view($view_name);
    if ($view && $view->access($display)) {
      $output = $view->preview($display, $view_args);
    }
    // This node is "safe" again.
    if ($node->nid) {
      _viewfield_nodestack_pop();
    }
    if (count($view->result)) return $output; 
  }
}
japanitrat’s picture

FileSize
910 bytes

Taking the code above into the module, the following patch should fix the problem

japanitrat’s picture

FileSize
911 bytes

err, wrong patch. here is the correct one

japanitrat’s picture

FileSize
935 bytes

good lord, i really need some sleep ...

(yes this is the correct patch now)

sun’s picture

+++ viewfield.theme.inc	20 Aug 2010 07:14:18 -0000
@@ -21,13 +21,17 @@
+      $output = $view->preview($display, $view_args);

Not sure whether I agree with this. Why would we want to invoke the view's preview if we want to embed it?

Powered by Dreditor.

muhleder’s picture

That piece of code $output = $view->preview($display, $view_args); comes from the Views function.

This patch basically replaces a call to views_embed_view with the code from views_embed_view, but includes a test on the number of results and returns nothing if number of results is 0.

views_embed_view otherwise returns an empty div tag (from memory), which cck then interprets as a non-empty field.

Here's the code from Views so you don't have to hunt it down

/**
 * Embed a view using a PHP snippet.
 *
 * This function is meant to be called from PHP snippets, should one wish to
 * embed a view in a node or something. It's meant to provide the simplest
 * solution and doesn't really offer a lot of options, but breaking the function
 * apart is pretty easy, and this provides a worthwhile guide to doing so.
 *
 * Note that this function does NOT display the title of the view. If you want
 * to do that, you will need to do what this function does manually, by
 * loading the view, getting the preview and then getting $view->get_title().
 *
 * @param $name
 *   The name of the view to embed.
 * @param $display_id
 *   The display id to embed. If unsure, use 'default', as it will always be
 *   valid. But things like 'page' or 'block' should work here.
 * @param ...
 *   Any additional parameters will be passed as arguments.
 */
function views_embed_view($name, $display_id = 'default') {
  $args = func_get_args();
  array_shift($args); // remove $name
  if (count($args)) {
    array_shift($args); // remove $display_id
  }

  $view = views_get_view($name);
  if (!$view || !$view->access($display_id)) {
    return;
  }

  return $view->preview($display_id, $args);
}
sun’s picture

Status: Needs work » Fixed
FileSize
1.1 KB

Thanks for reporting, reviewing, and testing! Committed attached patch.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Status: Fixed » Closed (fixed)

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

fraweg’s picture

Hello,

is there also a solution for drupal7?

Best regards
Frank

dropfen’s picture

Version: 6.x-1.x-dev » 7.x-2.0
Status: Closed (fixed) » Active

Bug is still present in 7.x !!!

SilviuChingaru’s picture

Status: Active » Needs review
FileSize
2.44 KB

If we approach this way, it needs a lot of code cleanup, but for an initial patch I'd like to show where the problem is at first.

SilviuChingaru’s picture

Cleaned up unnecessary code.

dman’s picture

Status: Needs review » Reviewed & tested by the community

+1 for me.
Seems to have fixed the problem on my limited testing.

mel-miller’s picture

Patch in #16 works for me. Thanks.

mvwensen’s picture

Patch in #16 works as a charm! Patch applied to version 7.x-2.0

holdmann’s picture

Some bug occured when applying patch #16.

Steps to reproduce:

1. Create view of single node type (e.g. articles) with contextual filter [Content:nid] from url, with option exclude argument from view result. Set style plugin as rendered entity, for example teasers;
2. Create viewfield field in node type, and attach view described above to it;
3. Remove viewfield display on teaser view mode. So this field shows only on full content.

That's all. Now there is WSOD with message "allowed memory exceed...". On my server allowed memory size is about 2Gb, so it looks like memory leaks.

I guess there is infinite loop - on hook_field_prepare_view(), but haven't chance to proove it.

Any ideas?

sashkernel’s picture

I'm using 7.x-2.0 and FieldGroup module. View field still returns something that causes FieldGroup to show the tab, even though it's supposed to invisible, because View returns nothing.
Any Idea how this can be fixed?

SilviuChingaru’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

@sashkernel please provide the viewfield output (at least html one) to figure out where problem is. I think your view is still outputing something. A view export will be also useful.

SilviuChingaru’s picture

Title: View field work not correct with empty views. » View field not working correctly with empty views.
Version: 7.x-2.0 » 7.x-2.x-dev
sashkernel’s picture

I've noticed that in Field Group module where when nothing is returned field group does not appear. This is the only place I see something returned.

This could well be related to the issue where I'm trying to pass an argument to the view through the view field and it's not working properly (https://drupal.org/node/1350916)

Here's HTML output I'm getting.

lass="field-group-format-toggler accordion-item ui-accordion-header ui-helper-reset ui-state-active ui-corner-top" role="tab" aria-expanded="true" aria-selected="true" tabindex="0">

    <span class="ui-icon ui-icon-triangle-1-s"></span>
    <a href="#" tabindex="-1">

        Shortcuts

    </a>

</h3>
<div class="field-group-format-wrapper group-shortcuts field-group-accordion-item ui-accordion-content ui-helper-reset ui-widget-content ui-corner-bottom ui-accordion-content-active" role="tabpanel" style="display: block;">

    <div class="field field-name-field-shortcuts-pane field-type-viewfield field-label-hidden">
        <div class="field-items">
            <div class="field-item even"></div>
        </div>
    </div>

</div>

Basically I'm not getting anything there, while in other viewfields that display view without contextual filters display correctly or simply disappear when view is blank.

SilviuChingaru’s picture

- Did you applied the patch from #16???
- Did you cleared cache and views cache?

sashkernel’s picture

#16 fixed it. Thank you fiftyz.

Is this going to be ported into the next version?

sashkernel’s picture

Continuing into this issue but for "Role Based view permissions".
I noticed that for views, which are rendering output only for particular role, "ViewField" does not work either. "Field Group" module detects that there is some output from "ViewField". And user who is not part of the permitted role sees the field group tab with blank output.
Pretty much same issue, but with role based view permissions.

jferjan’s picture

subscribing

rjacobs’s picture

Status: Postponed (maintainer needs more info) » Needs review

This was marked "Postponed (maintainer needs more info)" in #22, but it looks like the info requested was provided and deemed unrelated. I hope to not be stepping on anyone's toes, but I just wanted to update the status to reflect the current situation as best I can.

This was previously RTBC, but I'm not sure if if should be moved back to that status due to the concern raised in #20. Moving back to "Needs Review" for now.

For what it's worth we have been using this patch in production for months now without issues.

rjacobs’s picture

Hiding more outdated files.

keva’s picture

The patch in #16 works on the 2013-Oct-19 dev version.
Not using it with FieldGroup, so can't speak for that issue.

bfr’s picture

I get Fatal error: Call to undefined method stdClass::preview() in XXX/sites/all/modules/contrib/viewfield/viewfield.module on line 120
when i try to move Views cache to Redis with this patch applied. I don't think the class should be stdClass, maybe it's getting broken or null response when trying to load the view?

bfr’s picture

Seems like
$view = views_get_view($view_name);
sometimes returns null when Redis is used, and then
$view->override_path = current_path();
turns it into a stdClass object. Then ->preview() naturally crashes.

Checking for correct class seems to fix it:

  if ($view instanceof 'view') {
    $view->override_path = current_path();
    $view->preview($view_display, $args);
  }

but does this have side effects?

nasi’s picture

#16 works for me, patched against 7.x-2.0 (also not using field group).

sgurlt’s picture

Applied patch #16 against latest dev, looks good :)

seworthi’s picture

Status: Needs review » Reviewed & tested by the community

Works great.

sgurlt’s picture

Status: Reviewed & tested by the community » Needs work

Be careful with that patch, I just found out that when you are using rendered entities in a view and including a pager, this patch will break your pagers completly. I will investigate in this.

sgurlt’s picture

Status: Needs work » Needs review
FileSize
6.21 KB

Ok i rerolled patch #16.
I had two issues with it:

1. When a repeatable imagefield (with for example max repeat set to 5) was displayed inside the view, the view results wasnt empty even if no image was added at all. I improved the view result checking.

2. The second thing was a bit more tricky.
Imagine you have a viewfield attached to the user entity, this view field always shows (view-1). This (view-1) has a pager.
Then you have another view (view-2) which shows all users as rendered entites and you want to add a pager to this view. The pager will not be shown at all!
The problem is that when loading the rendered entity, the view from the user entity is also loaded, the pager ID of (view-1) is set globally and if the pager is currently not shown in (view-1), it is also will not be shown on (view-2). To fix that we simply can change the pager ID of (view-1) or (view-2), this can be done when editing the pager, just set it to another ID then 0.
I added a comment for this inside the README.txt

Cheers

AaronBauman’s picture

FileSize
5.89 KB

Attached patch did not apply cleanly for me, does not have appropriate diff context.
Here's a re-name, re-roll

sgurlt’s picture

Please review the patch also functionally.

lquessenberry’s picture

#39 Patched properly with no errors for me, but I was using the patch to solve a problem in another issue which said that #16 would be the solution. Can anyone confirm that they are using a viewfield with Search API?

https://www.drupal.org/node/1705874

If so, can you determine that the above patch #39 worked for you as a better alternative to #16 (which wouldn't patch for me properly)?

Thanks. Just to confirm, #39 patched for me just fine.

Thanks,

Lee

AaronBauman’s picture

#39 is a re-roll of #16.

They should be considered equivalent.

lquessenberry’s picture

Thanks! Now I just have to see what I am missing on getting the field to show up in Search API for an index.

Anybody’s picture

#39 solves the problem perfectly for me and removes the empty div without side effects so far. +1 for RTBC!

thomas.frobieter’s picture

I agree, #39 is working fine. +1 for RTBC

roflcopterDorrie’s picture

#39 works for me as well :)
Actually I am getting a fatal error when the view tries to render.
Fatal error: Unsupported operand types on line 1415 of node.module.
I have workbench moderation, views and viewfield working together, not sure if that is relevant or not.
Only happens when I apply the patch.

Anybody’s picture

@roflcopterDorrie could you please send a debug backtrace to find out what happens technically? Otherwise it will be hard to solve your problem.

We really have to take this issue forward. It's imporant and too old to stay open furthermore.

jsantander’s picture

I've applied patch #36 cleanly
However, it seems to be detecting a non-empty view with only a "title" field as empty.
Looking at the code, I'm not sure about the logic of this code:

      $empty_view = TRUE;
      // When the view has more then one result, we always have something to print.
      if (count($view->result < 2)) {
        foreach ($view->field as $field_name => $field) {
          // Looking for the field field_name array if it is empty.
          if (!empty(reset($view->result)->{'field_' . $field_name})) {
            // If it is not empty, we do not have a empty view,
            // and we can break the foreach for performance.
            $empty_view = FALSE;
            break;
          }
        }
      }

      // If we don't have any results or an empty value for this item, unset
      // it's delta because is an empty row in fact.
      if ($empty_view && empty($view->empty)) {
        unset($instance_items[$delta]);
      }

For one thing, the first if is wrong... it has the < 2 inside the count():
if (count($view->result) < 2) {
if (count($view->result < 2)) {

Then, I don't know much about the view's api, but my guess is that $view->result contains an array with one element for each item returned by the view.
My guess is that the code tries to prevent a case where one row is returned, but with all its fields set to empty.... but (once fixed the above mistake) will consider any >2 result empty.

jsantander’s picture

Well, after some investigation, this is my version of the #39 patch.
Basically I've modified the code above to be:

      $empty_view = FALSE;
      
      // When the view has more then one result, we always have something to print.
      if (count($view->result) < 2) {
        $empty_view = TRUE;
        foreach ($view->field as $field_name => $field) {
          // Looking for the field field_name array if it is empty.
          if (!empty($field->get_value(reset($view->result)))) {
            // If it is not empty, we do not have a empty view,
            // and we can break the foreach for performance.
            $empty_view = FALSE;
            break;
          }
        }
      } 

      // If we don't have any results or an empty value for this item, unset
      // it's delta because is an empty row in fact.
      if ($empty_view && empty($view->empty)) {
        unset($instance_items[$delta]);
      }

The changes are:

  1. Fix the if with count
  2. Make $empty_view=FALSE by default
  3. Use view_handler_field::get_value() as a safe way of checking the value of all available fields
jmato’s picture

#49 works for me, but only with the 1st and 2nd fixes, the 3rd fix crash with the following unrecoverable error: "Can't use method return value in write context". #39 hide the field rather if the view was empty or not. Using 2015-Apr-09 dev version.

morbiD’s picture

Status: Needs review » Needs work

The patch in #49 is broken and fails to apply to either a clean 7.x-2.x, or to 7.x-2.x with #39 already applied.

Looks like @jsantander did a reverse diff or something, so the patch is actually trying to remove his code rather than apply it...

Edit: Applying the patch in reverse works, but adds whitespace errors in two places. Also, the last line of the patch adds a double semicolon after the return.

Edit 2: I get the same error as #50. It seems you can't use empty() directly on the return from view_handler_field::get_value(). It must be assigned to a variable first, which can then be passed to empty().

morbiD’s picture

Status: Needs work » Needs review
FileSize
6.13 KB

This patch simply fixes #49 so it actually applies properly and doesn't produce fatal errors on PHP < 5.5.

I also reworded some of the documentation added by the patch since it wasn't clear to me what it actually meant until I went back and read #38.

The patch seems to resolve the issue for me but I can't comment on how well it works with other modules like Field Group.

I think the @todo items in the patch are probably best left for separate issues so I haven't bothered to address them here.

  • sun committed 7d14b9c on 8.x-3.x
    #477244 by muhleder, japanitrat, sun: Fixed empty view result output.
    
    
Syndz’s picture

I've noticed that the above patches only work for field based views.

This is because there's a check if field contain values, though, a view that renders entities, does not have field values.
I've added some extra logic so that only field values are being checked if the field row plugin is being used.

Syndz’s picture

FileSize
6.28 KB

The patch in #54 is generating a lot of notices, here's a new patch.

frjo’s picture

Added check for empty view that uses entity display to #55.

Other than that #55 worked well for me.

Heorhi Lazarevich’s picture

Patch #56 works for me for fields and entity display with Vewfield version 7.x-2.1

jatorresdev’s picture

Patch #56 works for me. Use display suite with Viewfield 7.x-2.1, please add it to the next version

jimafisk’s picture

Status: Needs review » Reviewed & tested by the community

Patch #56 works great! Applies cleanly to 7.x-2.1.

VasyOK’s picture

FileSize
194.38 KB

viewfield
Patch worked only if field NID exist in view. Even if this field is not needed

drupalgin’s picture

Patch #56 worked for me.

anneeasterling’s picture

In order to get this to work on our site, this code:

 +      // If we don't have any results or an empty value for this item, unset
+      // it's delta because is an empty row in fact.
+      if ($empty_view && empty($view->empty)) {
+        unset($instance_items[$delta]);
+ 

needed to be changed to

 +      // If we don't have any results or an empty value for this item, unset
+      // it's delta because is an empty row in fact.
+      if ($empty_view || empty($view->empty)) {
+        unset($instance_items[$delta]);
+ 

Can someone please verify that the changed version was the intent?

Thanks!

jerdavis’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/viewfield.module
    +++ b/viewfield.module
    @@ -102,6 +102,66 @@ function viewfield_field_prepare_view($entity_type, $entities, $field, $instance
    

    This hook is not the correct place to build the view and check for results. This hook is fired regardless of whether or not the field is to be displayed for a given view mode. The end result is if your view is going to output a set of teasers, and one of the nodes being output as a teaser has a viewfield - this code is executed and munges with the node content producing a fatal error, even if the viewfield on the teaser within the parent view was never going to be displayed.

  2. +++ b/viewfield.module
    @@ -102,6 +102,66 @@ function viewfield_field_prepare_view($entity_type, $entities, $field, $instance
    +      $empty_view = FALSE;
    +
    +      // Check if view is empty.
    +      if (isset($view->style_plugin->row_plugin->plugin_name)) {
    +        switch ($view->style_plugin->row_plugin->plugin_name) {
    +          case 'fields':
    +            // When view has more then one result, we always have something to print.
    +            if (count($view->result) < 2) {
    +              $empty_view = TRUE;
    +              foreach ($view->field as $field_name => $field) {
    +                $field_value = $field->get_value(reset($view->result));
    +
    +                // Looking for the field field_name array if it is empty.
    +                if (!empty($field_value)) {
    +                  // If it is not empty, we do not have a empty view,
    +                  // and we can break the foreach for performance.
    +                  $empty_view = FALSE;
    +                  break;
    +                }
    +              }
    +            }
    +            break;
    +
    +          case 'entity':
    +            if (empty($view->result)) {
    +              $empty_view = TRUE;
    +            }
    +            break;
    +        }
    +      }
    

    I'd like to see this reworked to not rely on style plugins. At this point in code execution checking $view->result should be sufficient. Checking each style plugin isn't sustainable as you'd need to account for all possible style plugins. As it is, this code only checks fields and entity, which in D7 would be node for full content.

jerdavis’s picture

Attaching a patch that adds a post_render callback to the top-level elements array. This callback iterates over any element children and checks their referenced view for results or an empty result behavior. If any of the child views have content the $content is returned. If they are all empty, NULL is returned.

Tested this against both field and content display styles. This update will be committed to 7.x-3.x today. I'll defer committing to 7.x-2.x pending further feedback.

jerdavis’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev
Status: Needs work » Fixed
jerdavis’s picture

Version: 7.x-3.x-dev » 7.x-2.x-dev
Status: Fixed » Needs review

  • jerdavis committed 2aac674 on 7.x-3.x
    Issue #477244 by japanitrat, fiftyz, Syndz, sgurlt, sun, morbiD,...
.bert’s picture

I can confirm that the patch in #64 applied cleanly to version 7.x-2.1 and resolved the issue in our use case.

We are also using field_group in our setup and there appears to be no issues.

  • jerdavis committed 6a75b0b on 7.x-2.x
    Issue #477244 by japanitrat, fiftyz, Syndz, sgurlt, sun, morbiD,...
jerdavis’s picture

Status: Needs review » Fixed

Thanks for the review! Pushed to 7.x-2.x to be included in the next release.

Status: Fixed » Closed (fixed)

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