2ND file uploaded is displayed when display option is unchecked

subscribe

Simple fix. Field items were being rekeyed before loop to remove hidden items finished.

Nice catch.

Could we please add tests for this? We've found all kinds of these tweaky edge-case bugs in File.module lately, and I'd like to ensure we don't ever have to fix them twice. :)

It looks like there was a test case to test the "display" option, but it only accounted for one file attached to a node. I'll rework the test so more than one file upload is tested. Do you think two will be enough?

Here's a reworking of the file field test to test multiple file fields (currently it's two, but could easily be changed to more). The only other thing I can see that needs to be tested is file order. It looks like there's a nice TODO message in field.test to add a test for display order, so maybe that can be handled in another issue.

Marked #1016068: File field display consecutive files hide error and #1041508: Problem with file_field_prepare_view as duplicates of this issue.

Confirmed this fixes a fairly major bug in D7 and the test looks like it covers it well.

Awesome, thanks!

I committed #3 to HEAD for now so we can get the bug fixed, but some feedback on the test:

-    $type_name = 'article';
+    $nid_or_type = 'article';

Ick! Can we not do this? Let's make separate variables for $nid and $type and use them accordingly. This change makes the test much harder to read and understand.

Agreed that $nid_or_type is just plain icky :). I was just referencing the call to FileFieldTestCase::uploadNodeFile, which takes $nid_or_type as an argument:

  /**
   * Upload a file to a node.
   */
  function uploadNodeFile($file, $field_name, $nid_or_type, $new_revision = TRUE) {

That function should also be rewritten, probably split out into two methods.

Well, we unfortunately can't change that function signature in Drupal 7 without breaking other peoples' tests. However, we do have control over what the variables are named within the tests calling that function. So let's just revert the name change back to what it was before.

So what's going to be done with this issue?

I've applied the #7 patch against Drupal 7.0 and it seems to resolve the issue.

Glad I found this. Client was extremely confused this morning. Patched 7.0 (using -p0) using #7 above and it works fine.

Works ok for Drupal 7.
(I only tested the two lines in the foreach, not the test stuff)

This still a concern in D8? Unassigned issue too.

The original bug was fixed back in 2011, and UploadNodeField, while it still exists, no longer has an $nid_or_file argument.