Make WordPress Core

Opened 11 years ago

Closed 9 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 10 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 10 years ago.
22768.3.diff (2.6 KB) - added by SergeyBiryukov 9 years ago.
22768.test.patch (1.0 KB) - added by bendoh 9 years ago.
unit test that checks caption goes to post_excerpt and not post_content
22768.4.diff (3.6 KB) - added by SergeyBiryukov 9 years ago.
22768.5.diff (4.7 KB) - added by SergeyBiryukov 9 years ago.

Download all attachments as: .zip

Change History (30)

#1 @jond3r
11 years ago

  • Cc jond3r@… added

#2 @sbressler
11 years ago

  • Cc sbressler@… added

#3 @DrewAPicture
11 years ago

  • Cc xoodrew@… added

#4 @kraftbj
10 years ago

  • Cc bk@… added

#5 @sbressler
10 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
10 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
10 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
10 years ago

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

@ericlewis
10 years ago

#8 follow-up: @ericlewis
10 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
9 years ago

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

#10 @RDall
9 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.


9 years ago

#12 @SergeyBiryukov
9 years ago

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

#13 @RDall
9 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.


9 years ago

#15 @SergeyBiryukov
9 years ago

#7580 was marked as a duplicate.

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


9 years ago

@bendoh
9 years ago

unit test that checks caption goes to post_excerpt and not post_content

#17 @bendoh
9 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
9 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
9 years ago

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

#20 @RDall
9 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 9 years ago by RDall (previous) (diff)

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


9 years ago

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


9 years ago

#23 @SergeyBiryukov
9 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
9 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.