Make WordPress Core

Opened 12 years ago

Closed 10 years ago

#22768 closed enhancement (fixed)

EXIF/IPTC captions should populate caption/post_excerpt on upload, not description/post_content

Reported by: nacin's profile nacin Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 4.2 Priority: normal
Severity: normal Version: 2.5
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description

It's been this way forever, but it is dumping data into the wrong field. Two schools of thought on this:

  • Plugins have used the description field, so we should not change this.
  • Users continue to struggle with moving *captions* from the description field to the caption field, so we should change this.

I am in the latter school of thought. A plugin can remain compatible by ensuring it checks either description or caption before falling back to the other.

Attachments (6)

22768.diff (1.1 KB) - added by beaulebens 11 years ago.
Moves 'caption' from EXIF/IPTC information into the post_excerpt, which is used for the caption field. Also removes one extraneous $content=;
22768.2.diff (3.0 KB) - added by ericlewis 11 years ago.
22768.3.diff (2.6 KB) - added by SergeyBiryukov 10 years ago.
22768.test.patch (1.0 KB) - added by bendoh 10 years ago.
unit test that checks caption goes to post_excerpt and not post_content
22768.4.diff (3.6 KB) - added by SergeyBiryukov 10 years ago.
22768.5.diff (4.7 KB) - added by SergeyBiryukov 10 years ago.

Download all attachments as: .zip

Change History (30)

#1 @jond3r
12 years ago

  • Cc jond3r@… added

#2 @sbressler
12 years ago

  • Cc sbressler@… added

#3 @DrewAPicture
12 years ago

  • Cc xoodrew@… added

#4 @kraftbj
11 years ago

  • Cc bk@… added

#5 @sbressler
11 years ago

Perhaps I'm being a big crazy here, but could we populate post_author from EXIF if there's an exact match?

@beaulebens
11 years ago

Moves 'caption' from EXIF/IPTC information into the post_excerpt, which is used for the caption field. Also removes one extraneous $content=;

#6 @helen
11 years ago

  • Keywords has-patch added

Didn't realize there was a patch here. What else is needed? Testing? Do we want to talk about any other fields?

#7 @ericlewis
11 years ago

This is technically a dupe of #7580, there's been more discussion going on there. Should we close this or that?

@ericlewis
11 years ago

#8 follow-up: @ericlewis
11 years ago

In attachment:22768.2.diff, while normalizing EXIF data, ensure that caption gets set. Formerly, the title would steal the caption field if the caption field was less than 80 chars. This logic still exists, but also sets the caption field.

#9 @RDall
10 years ago

  • Keywords needs-refresh needs-testing added; 3.6-early removed

#10 @RDall
10 years ago

  • Keywords needs-patch added; has-patch needs-testing removed

The patch as it currently stands can't even be applied because the file wp-includes/image.php doesn't even exist anymore.

I feel like I am yak shaving with this ticket. But with a ticket and a feature request this old it's to be expected.

This ticket was mentioned in Slack in #core by rdall. View the logs.


10 years ago

#12 @SergeyBiryukov
10 years ago

  • Keywords has-patch needs-unit-tests added; needs-refresh needs-patch removed
  • Milestone changed from Future Release to 4.2

#13 @RDall
10 years ago

I tested the patch ( - the unit testing ) it correctly imported the IPTC caption into the caption field. I leave the unit testing to the core developers. I am just glad were making progress on this.

This ticket was mentioned in Slack in #core by rdall. View the logs.


10 years ago

#15 @SergeyBiryukov
10 years ago

#7580 was marked as a duplicate.

This ticket was mentioned in Slack in #core by rdall. View the logs.


10 years ago

@bendoh
10 years ago

unit test that checks caption goes to post_excerpt and not post_content

#17 @bendoh
10 years ago

  • Keywords needs-unit-tests removed

Attached a unit test for this, looks good. It end up testing a lot of code as this behavior is found at the end of media_handle_upload.

#18 in reply to: ↑ 8 @SergeyBiryukov
10 years ago

Replying to ericlewis:

In attachment:22768.2.diff, while normalizing EXIF data, ensure that caption gets set. Formerly, the title would steal the caption field if the caption field was less than 80 chars. This logic still exists, but also sets the caption field.

22768.4.diff does the same for the Exif branch. Note that it's possible to have a duplicate title and caption with the new logic.

#19 @SergeyBiryukov
10 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#20 @RDall
10 years ago

As requested I have tested the patch on a version of WordPress running trunk with the various gallery plugins with a follow results.

NextGen - No issue, both photo and captions imports properly.

JetPack Tiled Gallery - No issue, both photo and captions

Rocket Galleries - No issue both photo and caption imported properly.

Image Gallery (Huge It) - Plugin falls to upload images properly in 4.2-alpha-31484 regardless of IPTC patch or not. The plugin performed poorly running on 4.1.1 so our patch will have little impact on this plugin in my opinion.

Gallery Bank - Plugin didn't upload IPTC information into caption on upload. But the patch doesn't break the plugin nor does it add any functionality either.

It would conclude (conjecture) that the plugins which use core functionality to include captions already found in the IPTC field is just applying the caption field properly. But the plugins that don't upload caption information before the applied the patch wouldn't apply the caption after the applied patch.

Last edited 10 years ago by RDall (previous) (diff)

This ticket was mentioned in Slack in #core by rdall. View the logs.


10 years ago

This ticket was mentioned in Slack in #core by rdall. View the logs.


10 years ago

#23 @SergeyBiryukov
10 years ago

  • Keywords commit added

22768.test.patch didn't work for me as is due to a PHP bug.

22768.5.diff is a combined patch with the revised test.

#24 @SergeyBiryukov
10 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 31694:

EXIF/IPTC captions should populate Caption (post_excerpt) on upload, not Description (post_content).

Make sure the caption is always set if found. Previously, if the caption was less than 80 characters, only the Title field would be set.

props beaulebens, ericlewis, bendoh, SergeyBiryukov.
fixes #22768.

Note: See TracTickets for help on using tickets.