If you add you have code that does this:

function mymodule_file_load(array $files) {
  foreach ($files as $file) {
    $file->alt = 'Default alternate text for this image';
  }
}

Then if that file is used in an image field, and the user manually provides an alterate text value in the field widget, that means that the node is loaded, the image field value's $item['alt'] gets overridden with $file->alt. Think the file entity properties should be merged, but not override any field value properties. It's a simple fix of re-ordering the array_merge() parameters in file_field_load().

CommentFileSizeAuthor
#2 2066275-before.png355.6 KBbrantwynn
#2 2066275-after.png353.81 KBbrantwynn
#1 2066275-file-field-load-merge-order.patch513 bytesDave Reid
PASSED: [[SimpleTest]]: [MySQL] 40,396 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

Dave Reid’s picture

Status: Active » Needs review
FileSize
513 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,396 pass(es). View

This does not need to be fixed in Drupal 8 because file fields set $item->entity as the file entity in that case, so there is no conflict between properties.

brantwynn’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
353.81 KB
355.6 KB

Tested this using the default 'Article' bundle with field_image. I added a new node with an image and entered alt text.

Before applying the patch, I was not seeing the any alt text on the rendered image on the page:
2066275-before.png

However, I was seeing the alt text I had entered in thefield_image_alt column in the field_data_field_image table in my db.

After applying the patch, I am now seeing the rendered alt text on the page:
2066275-after.png

This looks good to go.

Dave Reid’s picture

Assigned: Dave Reid » Dries

Dries asked this to be assigned to him at MWDS.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x. Thanks! :)

effulgentsia’s picture

Assigned: Dries » Unassigned

Status: Fixed » Closed (fixed)

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

theMusician’s picture

This seems to have reappeared in 7.24. The same patch, though now on line 189 fixes the problem.

quicksketch’s picture

It looks like this change is actually in the latest 7.x Git history, so it hasn't been lost. But because this is a "security release" only, 7.24 is identical to 7.23 (released August 7, 2013) other than the security fix. So everything is officially as it should be, but it's confusing to some users who have been waiting since August to have this fixed. The bottom line is, "wait for 7.25" (assuming it's not another security-only release).

theMusician’s picture

Thank you for addressing this so promptly quicksketch. I was not aware that security releases did not include bug fixes. I'll be running a patched version of 7.24 for peace of mind until 7.25 lands.

quicksketch’s picture

I just had the same experience, so I'm glad to have been able to help. :)

wiifm’s picture

Patch in #1 fixes all the things.

We had image galleries that utilised the title text for the caption, unfortunately Drupal decided to clobber 100% of this text (and the alt text) upon node_save(), meaning the content authors now have to manually recreate after this is patch released.

Wish this made it into 7.24. Data loss is never fun.

Vacilando’s picture

Also lost all image title and alt information with the update to Drupal 7.24 on several websites. This simple patch fixes it, but I despaired for a number of hours before finding it.

It's unfortunate that a normal (not just security) update has not been published since August 7th.

theMusician’s picture

Here is a patch file with the update applied to line 189.

https://bitbucket.org/wwuweb/collegesites/downloads/fix_file_attribute_c...

I too hope this gets amended in a Drupal 7.25 release.

David_Rothstein’s picture

Issue summary: View changes
Issue tags: +7.25 release notes

Added this issue to CHANGELOG.txt since it's a (minor) behavior/API change.

David_Rothstein’s picture

Title: file_field_load() overwites any field item properties with file entity properties » (followup: tests) file_field_load() overwites any field item properties with file entity properties
Status: Closed (fixed) » Active

It's also a testable bug but was committed without tests.

gonz’s picture

Just verified that the core patch did get committed to 7.25 - Just wanted to thank you @Dave Reid

dtisom’s picture

This patch worked for me. I went into my Drupal installation directory and ran

patch -p1 < 2066275-file-field-load-merge-order.patch

Be sure to make a backup of your original modules/file/file.field.inc in case it breaks something!

Chris Charlton’s picture

Is the only remaining component for this to move forward supplying missing tests?

bosmaen’s picture

Seems this issue has reappeared. I can't save Alt and Title text for images anymore. Running Drupal 7.56 and File Entity 7.x-2.11 and I can see that recommended patch above has been applied since 7.25 thereabouts.
Help.

divyesh19’s picture

This issue has re-appeared for me too.
Running Drupal 7.56 and File Entity 7.x-2.11.

Chris Charlton’s picture

I'm seeing this, too. Using current 7.x stable.

David_Rothstein’s picture

Title: (followup: tests) file_field_load() overwites any field item properties with file entity properties » file_field_load() overwites any field item properties with file entity properties
Status: Active » Postponed (maintainer needs more info)

I wrote a patch to provide the missing tests and posted it at #2933564: Write tests to ensure that file_field_load() doesn't overwite field item properties with file entity properties - please review. The test I wrote passes, so the bug that this issue was trying to fix certainly seems like it is fixed.

For those of you who say that you are still experience "this issue", what is the issue that you're experiencing exactly?

divyesh19’s picture

@David, I'm not able to save Alt text for images.
Upon further testing, I found out that this is working fine with File Entity 7.x-2.2. Facing this issue on File Entity 7.x-2.11.

David_Rothstein’s picture

Status: Postponed (maintainer needs more info) » Fixed

That sounds like #2918049: Image field alt and title text overridden by file_entity_alt and file_entity_title, which has nothing to do with Drupal core. Try updating to File Entity 7.x-2.12 to get the fix. (Not sure if that's what you meant when you wrote "7.x-2.2" i.e. maybe that was supposed to say "7.x-2.12").

I'm going to close this issue for now, but if anyone has any indication that there is an actual regression in the Drupal core behavior, feel free to file a separate issue with details and link to it here.

Status: Fixed » Closed (fixed)

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

alesr’s picture

Now after this patch is included in the latest release you can remove "Known Issues" paragraph from the File Entity project page.

David_Rothstein’s picture

@alesr, this was fixed in Drupal 7.25 which was released over 4 years ago, so yes, it's safe to say the File Entity module can remove that note from their project page :) I suggest filing an issue in the File Entity queue to do that, if there isn't one already.