Imagefield formatters don't respect visibility options

subscribe

same issue here

If I change the setting to

Default list value: * to hidden and after saving it auto reverts to listed. Same with the enforced/user configurable. It is automatically reverting to the user configurable.

Can anyone explain what this is anyways?

"The list option determines whether files are visible on node views. This will be used as the default value for the list option."

I don't want this List checkbox showing up on my upload form. It's ugly and I don't need it. I want the file showing up no matter what!

video: http://www.utipu.com/app/invited/id/2c881ac5a53f4588a0f8ffa71b96fcb1

amsgwp, i'm not seeing what so hard about it. select "Listed" and then select "Enforce Default". They'll always be listed and uses aren't presented with an option.

I'm not sure why you're settings aren't sticking. That's pretty odd.

I confirm this bug.... I have unchecked the listed for some images and they are shown in the node... if I put "Generic files" as the display mode they are hidden. But I want to use the image preset.

another bug I have is that When I add dynamically more fields with the "add another" button.. the List checkbox is not selected or checked by default.

The list checkbox has been fixed and improved in #388196: Revise the Field Configuration Options.

if I put "Generic files" as the display mode they are hidden. But I want to use the image preset.

This is because ImageCache does not respect the "list" option. We've considered making it respect it, but adding the option now seems like it would likely break a lot of sites that assume all the images are going to show up.

I think the best solution really would be to remove the "list" option entirely from ImageFields and have them always default to listed. We can leave this option in for FileField, but seems like the number of users that actually want this feature are extremely small, it's there currently because it's a side-effect of building on top of FileField.

I really think image cache should respect the list option.
there could be an option in the administration section with a button to toggle all imagefields list option on /off quickly for all fields in database.

Marked #305238: List Check Box behavior as duplicate.

Subscribing. I too would prefer Imagecache respecting the list value. There are scenarios where you want users to be able to choose.

But the current situation is confusing, so if it's not possible to have ImageCache respect the setting (or not desirable for most people), it makes sense not to have the list option for imagefields.

I would prefer imagecache to respect the list value also.

There are numerous use cases where this makes the most sense. Especially with solutions like: http://drupal.org/project/filefield_insert . When you insert an image it shows up from the file/imagefield and in the node body.

At Zivtech we'd be willing to create the patch or whatever it takes to get this working. What roughly needs to be done? Should it remain a patch or be integrated into the module, or a small separate module?

acouch, thanks for volunteering to create a patch. There a couple options available.

We could attempt to move processing of the "list" option from the theme_filefield_item() to filefield_field_sanitize(). This would make a single point of change that would immediately affect all FileField-based modules, including all the formatters provided by ImageField and ImageCache.

Or, we could update the formatters provided by ImageField and ImageCache to do the same check as theme_filefield_item().

Regardless of the approach we take, an _update function must be provided that ensures that files don't suddenly stop (or start) displaying after upgrading to the new version. It's a very dangerous move, since it's unlikely that we'll be able to not break sites considering the lax use of the list option previously. While I agree that it'd be nice for ImageField and ImageCache to obey the "list" option, I don't know if it's feasible to make this change now. The last thing we'd want is for users to update an all the images on their site to disappear because the list option is enabled but unchecked on all their files (a very common combination unfortunately).

Cool. We'll review and let you know if there is an approach that makes the most sense to us.

If an update function sets all imagefiled images to 'listed' then folks would be starting where they left off since all of the images were visible previously. Right?

If an update function sets all imagefiled images to 'listed' then folks would be starting where they left off since all of the images were visible previously. Right?

That's mostly correct. You'll need to check which formatter was being used when displaying the field and set "list" = 1 when any formatter was being used other than the ones provided by FileField.

Actually, not only does Imagecache fail to respect the "list" checkbox but imagefield ignores it as well. I have attached patches for each that cause them to respect the the list item but did not yet tackle the problem quicksketch pointed out with creaing an _update function to prevent us from breaking sites because I think it is a bit more complicated than he indicated in the previous comment.

A content type could be set to use generic files in the teaser but full in the body and that doesn't even take into consideration views. Perhaps we could loop through each field on each content type and each view making sure that nowhere in the site is there a conflict and then proceed to set the list value to 1, but even then we'll need some kind of an interface to explain the problem and to inform the user of which nodes have conflicts that need to be resolved. Is it more important to ensure that we don't have a broken interface element in an important module or not to break sites? This is a pretty rare case and the list checkbox is off by default and even then list is enabled by default. I think this probably affects a relatively small group of users. Tough call...

Also, each patch adds a (slightly modified) copy of filefield_file_listed from file field's filefield_formatter.inc. It might be a good idea to move this function into a file that is always included (filefield.module maybe?) to prevent the redundancy.

A couple things that might improve these patches:

- Use the shortcutted operator "||" instead of "|", there's a slight (very, very slight) performance improvement that the second condition is never checked if the first one fails. More importantly it matches code standards.
- ImageField fully depends on FileField, so you can just use filefield_file_listed() instead of making a duplicate function.
- You should use isset() when checking values such as $field['list_field'], to prevent PHP notices.

Of course a update paths are needed also, including display settings stored by CCK and formatters within Views. Unfortunately there's nothing that you can do about code-based default views (a common practice among shops that store views in code for version control) other than give them a warning. But for such circumstances usually just a notice in the release notes is needed.

Thanks for the effort so far, though I'm still not sure what to do with such patches upon completion. As you note it's a risky business, one that's not entirely clear whether the benifits of increased flexibility outweigh potentially breaking existing sites.

I realize that imagefield fully depends on filefield but the filefield_file_listed() function is in filefield_formatter.inc which I believe is only conditionally included by CCK? If I try to call that function directly I get "Fatal error: Call to undefined function filefield_file_listed()".

As for the isset(), that section was copied from filefield_formatter.inc. Should I file a patch to change that as well? I'm guessing that the thinking was that these formatters are always applied to fields created by filefield so it will always be set but perhaps that's too big an assumption?

As for upgrading, I hadn't thought of the issue with code based views. I have to imagine that the number of people relying on images ignoring the list setting is pretty small. These people have to go to the trouble of setting "list field" to enabled and then further uncheck "files listed" to even potentially have a problem.

It seems like the only people that will have trouble with the issue is if someone has two different display types and want the list_field check to work on only one of them (a generic list of select files on the teaser and all of the actual images displayed on the actual node, etc.). These users are likely fairly advanced, could a notice be sufficient for these limited cases?

It seems like a "conflict resolver" script would be a lot of work and legacy code for what might actually be an extremely limited use case.

setting status

These people have to go to the trouble of setting "list field" to enabled and then further uncheck "files listed" to even potentially have a problem.

I changed these default settings in FileField 3.0 beta1 about 3 months ago, it used to be completely opposite and the default was to have the list field enabled and not listed by default. :-P So it's likely there are thousands of sites that have that as the setting, considering it was the default for FileField in Drupal 6 for over a year. An upgrade path is absolutely a requirement for these patches both for CCK formatters and database views.

tizzo,

I realize that imagefield fully depends on filefield but the filefield_file_listed() function is in filefield_formatter.inc which I believe is only conditionally included by CCK? If I try to call that function directly I get "Fatal error: Call to undefined function filefield_file_listed()".

see module_load_include(), e.g. module_load_include('inc', 'filefield', 'filefield_formatter');

Thanks drewish! I hadn't run across that function before! I rerolled the patch using filefield's function.

I didn't realize unlisted had been the default. Would a fair update path be to loop through the display formatters for each field on each content type and on each view and set the list option to 1 if there is no conflict (all of the formatters used are either imagefield or imagecache) and then alert the user if there are conflicts while taking no action? I suppose that could break a number of sites though less.

The only alternative I see is to then set a variable in the variables table that a conflict has occurred and check for that before respecting the setting. This would require some kind of interface for conflict resolution (listing the node title in the table along with each file and a checkbox whether to list it or not)?

What is the situation with D7? With filefield and imagecache going into core will this problem be moved into core? Can we apply a patch to core without an upgrade fix? Or would that be too big an obstacle to add to the core upgrade path?

subscribing

Sorry for the delay.

Here is a patch with at least a first swipe at upgrade scripts. It errors on the side of showing files by looping through every content type and every view looking at each field in each and checking to see if the display field is != default (generic file). If it is a filefield and is displayed as something other than a generic file anywhere on the site all entries for that field are updated to list = 1.

I also added a variable_set and variable_get so that each module can see whether the other has run this update and abort if it has. This way if someone makes changes to the list settings after making these list options actually do something they will not loose their changes by applying another update.

Thoughts?

Giving this a bump, I think these patches solve the problem roughly as we've discussed it should be solved.

The use of variable_get/set is extremely odd here. Instead you should be using drupal_get_schema_versions($module) and checking the schema number if necessary. Otherwise we're permanently adding variables that will never be properly cleaned up. In addition, the patch still isn't quite following coding standards. Start comments with capital letters, and end with periods. Put a space between logic constructs and parenthesis if ($condition) not if($condition). Brackets are missing around table names in updates. Rather than using content_types and trying to manually assemble table names and field names, you should use content_types_install() to get a list of fields.

subscribing

subscribing

subscribing

subscribing

Hi,

any news on this issue?

For us who inline images in the body (I do that with #394682: CCK fields (filefield,imagefield,emfield) and fieldgroups for Inline API), this change is very important.

Thanks,
Antonio

I think it's unlikely that this bug will be fixed, since the Drupal 7 version of Image module doesn't even have the "Display" option, so this ability will be lost when upgrading to Drupal 7 anyway.

But there will be another way to control visibility of a field in D7, right? So an upgrade path could still handle this. And considering D6 will stay there for a while this issue could still have sense.

A question I have, can an upgrade procedure be interactive? I mean, could a module propose to the user two alternative upgrade paths to choose? (Display checked for all vs. Display unchecked for all).

Thanks,
Antonio

Having the visibility("list") option on the imagefield is VERY useful.

I am using file field images, one use is to allow the user to add images to a slideshow on their node by checking the "list" option next to the images when editing.

To workaround this bug:

• I turned off display of my image_field in the CCK
• I used Contemplate Module to override the body template with:

<?PHP echo $node->body ; ?>

<div class="field field-type-filefield field-field-images">
  <h3 class="field-label">Images</h3>
  <div class="field-items">
    <?php foreach ((array)$node->field_images as $item) { ?>
      <?php if ($item['list'] == 1) {      ?>
        <div class="field-item"><?php print $item['view'] ?></div>
      <?php } ?>
    <?php } ?>
  </div>
</div>

This checks the "list" property of each image before displaying it with the defined "view". And I output the body beforehand w/o this field.

But there will be another way to control visibility of a field in D7, right?

D7 will have the Content Permissions module (or more likely renamed to "Field permissions"), which allows per-field access. However D7 will not have the ability to show/hide individual images when uploaded through ImageField.

D7 will have the Content Permissions module (or more likely renamed to "Field permissions"), which allows per-field access. However D7 will not have the ability to show/hide individual images when uploaded through ImageField.

Ahhh, this doesn't sound very good :(
Slightly OT: What about Filefield? Will the per-item visibility option be removed there?

Slightly OT: What about Filefield? Will the per-item visibility option be removed there?

FileField still has per-item visibility and it will have an upgrade path (for both FileField and Upload module) from D6.

FileField still has per-item visibility and it will have an upgrade path (for both FileField and Upload module) from D6.

Then, forgive me, I cannot see why the asymmetry with Imagefield, being it in a IS-A relationship with Filefield.
I can understand the practical difficulties in upgrading the "List" option for current installations of Imagefield, but I still hope these will be tackled and resolved someway.

bump

Hi to all

I have the same problem with visibility behavior of "Listed" file in ImageField (last release).

Considering I'm creating a new website (so I don't need any upgrade) and the patches listed above: what solution is better for me? (Do I have to patch and what version to use?)

I need absolutely the "un-listed" feature to avoid duplicate image in node (and also with Lightbox integration) using Insert module to place image inside the node text.

Thanks a lot!
Bye.

Same problem here :
ImageField : 3.2
CCK : 2.6
FileField : 3.2

Subscribing

Can we have a comment from imagefield developers?

Will this be fixed? I was also wondering if the update could be made interactive to decide between "listed by default" and "unlisted by default" when updating existing imagefileds.

An alternative way to list/unlist imagefields would be ok of course.

Thanks,
Antonio

Will this be fixed?

No it will not be fixed, because it was never intended to work at all. This functionality has been removed entirely from the 7.x version of ImageField (which is now in core), meaning it would be impossible to provide an upgrade path. If you choose to try to use this functionality then it will certainly break again when upgrading to Drupal 7.

@quicksketch: Ok, I see. I let to you to mark this as "won't fix".

So it might be a better idea to move this to Drupal 7's issue queue.

Just for the record--i've worked around this issue--but when using something like Insert, this functionality would be nice to have. I have a case where I let users upload images and insert some into the body field, and I would like them to be able to check others to 'list' in a different field on the page. I've just created two different image fields, which works fine, but makes for a slightly more cluttered interface.

So it might be a better idea to move this to Drupal 7's issue queue.

Drupal 7 doesn't even have a "display" option for images, so it doesn't apply there. The fact that it will be impossible to upgrade to Drupal 7 is one of the reasons why there's no point in adding this in Drupal 6.

Moving this to a postponed support request. As stated above this functionality will not be added. You can work around it in your own theme, but support will never be added directly to ImageField.

quicksketch - I think there are either two ways to make the community happy and solve this issue.

A. Remove the options. Why are you presenting us options that do nothing?

B. Apply my patch or one of the others that have been linked here. It's two lines of code!

At this point you have spent more time arguing about it then it would have taken to fix it.

See #43. I won't change this functionality.

I don't understand why and this is incredibly frustrating. If you have no desire to make this feature work (which even if you didn't want to use the patches provided would require little effort on your part) you could remove these options so people do not spends hours of their time attempting to get something to work that two lines of code could easily resolve. Hell if you want me to I'll even take the time to submit a patch that removes them, I haven't looked at it yet but I'm sure it's nothing more then commenting out a few lines of code.

Basically you are saying "Run custom code on your sites if your clients want this. I could remove this option or add this feature in but despite community feedback I simply do not wish to. Oh and anyone who has yet to find this issue enjoy wasting a few hours of your time debugging the problem."

I apologize if I come off a bit rude but you have no idea how maddening this is. Am I missing something here? Is there some massive undertaking to creating a new version to of this module so that you NEVER have to hear about this problem again that I am not aware of?

The problem with these options is a matter of existing users (who may already be using the "display" and "description" options) and future use (since neither of these options will exist in Drupal 7). If we add support for these features we'll encourage use of fields that do not exist in the next version of ImageField (or "Image" in Drupal 7). If we remove these features we may be removing something people are already using. I NEVER wanted these options in ImageField (they weren't in the Drupal 5 version) and it just so happened that I was not responsible for the development of this module until Drupal 6 had been out for 18 months with plenty of people already using the module. The entire existence of them happens to be an accidental side-effect of building on FileField.

So, given the options of "add", "remove", or "do nothing" I've been taking the "do nothing" approach because "add" and "remove" both have down sides to existing users. The "do nothing" approach of course is not good for new users.

Simply commenting out a few lines of code (not removing) so that users do not see the options leaves us in a much better position. An accompanying small update to the readme saying "If you wish to use these features you will need to uncomment lines X-X and then do x, however please keep in mind this feature will not be available in Drupal 7" keeps everyone happy.

I can understand that it can be frustrating to have a project you have worked on go a direction you do not want it to go but leaving a feature in that does not work is just not a good option. Remember that Drupal is not just for the tech savvy, while this all makes sense to us someone who just casually uses Drupal is not going to understand this and just think Drupal is broken. This is exactly what happened with one of my clients who called up complaining that this feature was broken and sent me down this rabbit hole.

We lost hours on this issue today. Quicksketch, there has to be a way to remove this non-functioning list checkbox without affecting those who are relying on it. Durrok's comment seems to provide such a path forward.

Having a non-functional checkbox on the node form on a popular module means that Drupal 6 is just broken, and in a fairly visible way.

Why not have the checkbox clearly explained on the node form, with a full disclaimer on why it does not work and why it is still present. That way new users don't have to get frustrated and legacy users don't have to hack the module every update.

quicksketch - Thanks for taking my question at the SF Drupal meetup. I wasn't trying to be a jerk but I'm not all that great at speaking in public so sorry if I came off that way... and don't worry, the bill is in the mail. ;)

We use the http://drupal.org/project/conditional_fields module to control visibility of elements on the edit node form and hence in the node.

You can fix this in your theme for any preset by overriding the imagecache formatter theme functions. Grab the default formatter code from imagecache.module and wrap it in logic to check for the list field. For example, to override the default image-only formatter with a check for the list field, add this to your template.php...

function YOURTHEME_imagecache_formatter_PRESETNAME_default($element) {
  // Load the field definition for the current element
  $field = content_fields($element['#field_name']);

  // If the list field is enabled, and this element has list checked...
  if ($field['list_field'] == 0 || ($field['list_field']  && $element['#item']['list'])) {
    // Inside a view $element may contain NULL data. In that case, just return.
    if (empty($element['#item']['fid'])) {
      return '';
    }
  
    // Extract the preset name from the formatter name.
    $presetname = substr($element['#formatter'], 0, strrpos($element['#formatter'], '_'));
    $style = 'linked';
    $style = 'default';
  
    $item = $element['#item'];
    $item['data']['alt'] = isset($item['data']['alt']) ? $item['data']['alt'] : '';
    $item['data']['title'] = isset($item['data']['title']) ? $item['data']['title'] : NULL;
  
    $class = "imagecache imagecache-$presetname imagecache-$style imagecache-{$element['#formatter']}";
    return theme('imagecache', $presetname, $item['filepath'], $item['data']['alt'], $item['data']['title'], array('class' => $class));
  } else {
    return '';
  }
}

Note that it's per imagecache preset.

To override the linked formatter, apply the same if() block to an overridden YOURTHEME_imagecache_formatter_PRESETNAME_linked.

Encountered the same issue. Subscribing...

#49 works like charm! :)

Well I was tempted to patch as well. Can think of many reasons to have the list option working for formatted images. But if Nathan says no I guess it is no :) I just hope the option will not be removed cause even though it doesn't work out of the box with image formatters there is an easy workaround.

In addition to #58 I would like to add mine;

I added a tpl content-field-field_image.tpl.php to my theme and changed it to;

<?php if (!$field_empty) : ?>
    <?php foreach ($items as $delta => $item) :
      // add  $item['list'] == 1 to the if statement below
      if (!$item['empty'] && $item['list'] == 1) : ?>
      <div class="item"><?php print $item['view'] ?></div>
      <?php endif;
    endforeach;?>
<?php endif; ?>

Works for me!

Cheers

#62 works.
don't forget put original content-field.tpl.php in theme folder too

Closing as outdated, because drupal 6 is not supported anymore. I hope that's OK.