Posted by kaare on June 9, 2009 at 2:18am
| Project: | Exif |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Whenever i save a picture without EXIF info, the field field_ifd0_datetime_value is filled with the value '0000-00-00 00:00:00' in the database, and further, due to cck's handling of this, displayed as the current date. That's pretty weird.
Now, I dived into the date cck handling code and thought I found the bug, but apparently I did not. See #482436: Empty values aren't reset for delta = 0.
The problem is that the field value need to be 'NULL' if it is to be NULL in db, and further, not be displayed. So, this has to be fixed in exif.module. Patch attached.
| Attachment | Size |
|---|---|
| exif-empty-date.patch | 1.19 KB |
Comments
#1
can you please update this patch, so it works with the latest version of head, so that it can be testet? I'll then make sure it get's into the module
#2
Sorry for the delay.
Merely updating the patch isn't enough anymore due to the altered traversing of fields. When saving a node (insert, update) your algorithm now only saves the fields that have available exif data from the image. This is fundamentally flawed, especially when you change the picture to something with different or missing exif data. You need to loop through all available exif-enabled cck fields and set it's value to the value found in the exif header or NULL otherwise. The easiest scenario for this is when you replace the original image with one withouth exif headers. All exif cck values from the previous picture will now be stale upon updating the node. As no exif data was found, neither will any fields be updated. You need to update all fields.
If you agree with me on this I'll take a stab at a patch fixing this.
#3
I went ahead and created this patch anyway. This patch alters the way fields are traversed as described in my previous comment. In addition, it cleans up the handling of dates. I won't repeat the reasoning in my previous comment, but the date handling requires some notes:
Proper Date field handling
The value stored for date fields are a bit more complex than those as part of the cck package. I've tested this patch for all date types in date module (date, datetime, datestamp), with all available exif date values, and it works both for node creation and update. Dates are parsed, converted and saved, all according to settings set in Date API and date field settings. Timezones are respected, date granularity is respected (exif date granularity is now obsolete and should be removed), and stored date types are respected. It's all-in-all a far more robust handling of dates than the current version.
Removed unnecessary date cleanup in _exif_reformat()
The altering of dates in
_exif_reformat()was unnecessary. In the admin exif overview page it altered the presentation of dates found in the exif header. This needs no alteration. Inexif_nodeapiit never ran as the$keynever was found in$date_array. It was, on the other hand, duplication of code alreay found inexif_nodeapi.Date API exif format
The exif date format (
YYYY:MM:DD HH:MM:SS) is now available through the Date API, which lets you override the format exif dates should be parsed as. No immediate reason to override, but the opportunity is there. Note that this let's the administrator decide what format exif dates should be parsed as, not displayed as, so it might be more confusing than useful.Please review this patch.
#4
patch doesn't apply.
#5
try with
patch -p1 < exif-date_api.patch#6
sorry... didn't work. can you check that again or maybe rebuild the batch from the latest head
#7
Ok, here it is, using
cvs diff -duagainst DRUPAL-6--1. The HEAD version looks like it's for Drupal-5.x.#8
seems to be very messy in CVS. There's D6 and D5 stuff mixed in Head :( ... arg. I guess the 1.2 Version is the most acurate. Let me figure it out with David what's going on in the head.
#9
oky. ... this worked now. is committed to 6-1
#10
Automatically closed -- issue fixed for 2 weeks with no activity.