Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#50675 closed defect (bug) (fixed)

Media: migrate the data from the attachment post of the parent image when saving an edited image

Reported by: azaozz's profile azaozz Owned by: azaozz's profile azaozz
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.5
Component: Media Keywords: has-patch needs-unit-tests
Focuses: accessibility, rest-api Cc:

Description

When saving an edited image through the REST API (from the Image block in the editor), a new attachment post is created. This works similarly to uploading another version of the image. Currently only the EXIF data is migrated to the new attachment (if not recreated). In some cases it seems some or all of the parent image's attachment post and meta should also be migrated to the edited image's attachment.

Attachments (1)

50675.diff (2.4 KB) - added by azaozz 4 years ago.

Download all attachments as: .zip

Change History (15)

#1 @azaozz
4 years ago

This mainly concerns:

  • post_content (image description).
  • post_excerpt (image caption as saved in the DB).
  • _wp_attachment_image_alt (alt text for the img tag as saved in the DB).

Seems it may make sense to migrate the description and caption. Editing an already used image may require new caption and description, and migrating the old values may be somewhat unexpected in these cases. On the other hand it is still the same image...

Unsure about migrating the alt text though. According to W3C WAI the img alt must be contextial. Even storing it in the DB is "questionable" when inserting a previously used image in the editor, see #47456. Editing and reusing an image is quite likely to be in different context, and require new alt text.

Last edited 4 years ago by azaozz (previous) (diff)

#2 @TimothyBlynJacobs
4 years ago

  • Focuses rest-api added

@azaozz
4 years ago

#3 @azaozz
4 years ago

  • Keywords has-patch added

In 50675.diff:

  • Copy post_content and post_excerpt from the parent image attachment.
  • Fix merging EXIF image data from the parent image to the new image meta.

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


4 years ago

#5 @TimothyBlynJacobs
4 years ago

  • Keywords needs-unit-tests added

This looks good to me. Cc: @spacedmonkey.

#6 follow-up: @spacedmonkey
4 years ago

I am not sure what you mean here. At alt is used to describe an image. The current alt is doesn't care for context, why should we care now. I know that if an image is cropped the alt might be wrong, but I think for people using screen readers, something is better than nothing. I say we copy it over and then the user can edit what is there. I think this is a safer course of action, then not copying it at all.

What about the post_title? Should this be copied over to the new image as well?

#7 @spacedmonkey
4 years ago

We could also look at copying over the parent id from the old image as well.

This ticket was mentioned in Slack in #accessibility by spacedmonkey. View the logs.


4 years ago

#9 in reply to: ↑ 6 @azaozz
4 years ago

Replying to spacedmonkey:

I am not sure what you mean here. At alt is used to describe an image.

Not always. The alt text may be a description of the image in some cases but in other cases should be contextual, i.e. describe the image's meaning in the surrounding text. See https://www.w3.org/WAI/tutorials/images/decision-tree/. Also, the image alt may be used to "redirect" to the image caption, example: https://www.w3.org/WAI/tutorials/images/complex/#structurally-associating-the-image-and-its-adjacent-long-description-html5.

The current alt doesn't care for context

Right, #47456 is for fixing this. Seems better to not add more places that need fixing :)

Don't mind copying the alt text when present, but seems the chances that the old alt text is "wrong" are high.

What about the post_title? Should this be copied over to the new image as well?

Good point. This would create several posts with the same titles, i.e. the slugs will have -1, -2, etc. but if the parent image's title was edited, the user expectation would likely be to keep it.

We could also look at copying over the parent id from the old image as well.

Hmm, perhaps. Would prevent the need to use get_post() in a loop when needing the "grand-parent(s)" of an edited image. On the other hand that would make the parent_image an array of arrays, or perhaps can be an associative array $attachment_id => $relative_file_path,. Unsure if it's "worth the effort", how useful that would be.

Last edited 4 years ago by azaozz (previous) (diff)

#10 follow-up: @joedolson
4 years ago

Copying any of the information over at all requires a certain amount of assumption about the nature of the editing process; certainly for changes of size or rotations, it's still the same image - for cropping actions, that's less clear; it may or may not be the same image.

And I'd note, @spacedmonkey, that an alt attribute does *not* describe the image; it describes the purpose of the image. In some cases, that text might describe the image itself, but certainly not in all cases.

Ideally, I think that there should be a user interface to give people the opportunity to edit meta for an attachment at the time it's created; it could be pre-populated with the existing meta data, as even if the information has changed, that's a likely starting point.

#11 @afercia
4 years ago

  • Focuses accessibility added

#13 in reply to: ↑ 10 @azaozz
4 years ago

Replying to joedolson:

Copying any of the information over at all requires a certain amount of assumption about the nature of the editing process; certainly for changes of size or rotations, it's still the same image - for cropping actions, that's less clear; it may or may not be the same image.

Yes, went back and forth on this couple of times. Seems most users would expect the image description and caption to be the same after editing it. This is also the current case, hence the change.

Ideally, I think that there should be a user interface to give people the opportunity to edit meta for an attachment at the time it's created

Yes, lets try to do #47456 in 5.6. It' is a large change that touches many parts, both UI and "backend", and ideally will be done by a small team/several people.

...it could be pre-populated with the existing meta data, as even if the information has changed, that's a likely starting point.

Sounds good. Added in https://github.com/WordPress/wordpress-develop/pull/415.

#14 @azaozz
4 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 48510:

Media: Migrate the data from the attachment post of the parent image when saving an edited image. Copy:

  • post_title,
  • post_content (image description),
  • post_excerpt (image caption as saved in the DB),
  • _wp_attachment_image_alt meta (alt text for the img tag as saved in the DB).

Props spacedmonkey, joedolson, TimothyBlynJacobs, azaozz.
Fixes #50675.

Note: See TracTickets for help on using tickets.