Make WordPress Core

Opened 2 years ago

Closed 9 months ago

Last modified 9 months ago

#58051 closed defect (bug) (fixed)

Attachment custom fields not rendered after upload

Reported by: trepmal's profile trepmal Owned by: antpb's profile antpb
Milestone: 6.2.1 Priority: normal
Severity: normal Version: 6.2
Component: Media Keywords:
Focuses: accessibility, javascript Cc:

Description

This is a follow-up to #40909.

Attachment custom fields added via attachment_fields_to_edit are no longer rendered just after an image is uploaded.

Clicking away and back will restore the custom field markup, but is quite inconvenient.

Attachments (3)

58051-demo.mov (2.9 MB) - added by trepmal 2 years ago.
demonstration of behavior
58051.diff (585 bytes) - added by adamsilverstein 2 years ago.
test-custom-fields-upload.mp4 (9.5 MB) - added by adamsilverstein 2 years ago.

Change History (57)

@trepmal
2 years ago

demonstration of behavior

#1 @joedolson
2 years ago

  • Milestone changed from Awaiting Review to 6.2.1
  • Owner set to joedolson
  • Status changed from new to accepted

#2 @costdev
2 years ago

  • Keywords needs-patch added

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


2 years ago

#4 @joedolson
2 years ago

Possible ideas include reinitializing the frame after an upload, reverting the change in 40909, and providing the initialize method that was removed with context for where focus should be. But this needs further exploration.

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


2 years ago

This ticket was mentioned in PR #4315 on WordPress/wordpress-develop by @adamsilverstein.


2 years ago
#6

  • Keywords has-patch added; needs-patch removed

#7 @adamsilverstein
2 years ago

@joedolson what about something like 58051.diff which triggers the render on the add callback instead? I tested this (briefly) and I see the fields (immediately after uploading completes) - also the focus issue from #40909 still appears resolved.

@trepmal does this fix your issue or cause any unexpected side effects?

This ticket was mentioned in Slack in #core-media by adamsilverstein. View the logs.


2 years ago

#9 @joedolson
2 years ago

  • Keywords reviewing added

#10 @joedolson
2 years ago

  • Keywords commit added; reviewing removed

Test steps:

Failure case:

1) Set up custom fields using code sample on #40909
2) Go to add a featured image to a post.
3) Upload image by dropping into thumbnail grid within the media modal.
4) Verify that attachment fields do not display.

Success case:

1) Apply patch and build.
2) Go to add a featured image to a post.
3) Upload image by dropping into thumbnail grid within the media modal.
4) Verify that attachment fields do display.
5) Add content to test field 1; tab forward to observed focus. Observe that focus stays in field 2.

Tested patch and verified test results as expected.

Marking for commit.

This ticket was mentioned in PR #4322 on WordPress/wordpress-develop by @joedolson.


2 years ago
#11

Test against media changes to render attachment fields on new uploads.

Trac ticket: https://core.trac.wordpress.org/ticket/58051

#12 @joedolson
2 years ago

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

In 55649:

Media: Render attachment custom fields for new uploads.

Initialize attachment custom fields during the add callback, so that fields are present as soon as an attachment is uploaded but do not refresh when field values are changed. Follow up to #40909.

Props trepmal, adamsilverstein, joedolson.
Fixes #58051.

#13 @joedolson
2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#14 @joedolson
2 years ago

Reopened to backport to 6.2.1

#15 @bbreedlove
2 years ago

I'm encountering the bug described here and I wanted to pop in and note that I've tested the patch attached on this ticket and it successfully resolves the problem.

#16 @audrasjb
2 years ago

  • Keywords fixed-major added

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


2 years ago

#18 @joedolson
2 years ago

#58210 was marked as a duplicate.

#19 @audrasjb
2 years ago

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

In 55735:

Media: Render attachment custom fields for new uploads.

Initialize attachment custom fields during the add callback, so that fields are present as soon as an attachment is uploaded but do not refresh when field values are changed. Follow up to #40909.

Props trepmal, adamsilverstein, joedolson.
Merges [55649] to the 6.2 branch.
Fixes #58051.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


23 months ago

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


23 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


22 months ago

#25 @antpb
22 months ago

  • Keywords early added
  • Milestone changed from 6.2.1 to 6.4
  • Resolution fixed deleted
  • Status changed from closed to reopened

This issue seems to still be happening in certain frames. For instance if you upload an attachment using a custom media library button in something like a custom field, a newly uploaded item will not render the custom fields as expected. Moving this to 6.4 for investigation and a fix. I've been working on a fix for a bit now and am close. Marking early.

This ticket was mentioned in Slack in #core-media by costdev. View the logs.


22 months ago

#27 @joedolson
22 months ago

  • Owner changed from joedolson to antpb
  • Status changed from reopened to assigned

#28 @hellofromTonya
21 months ago

  • Keywords needs-patch added; has-patch commit early removed

I've removed the early keyword. Why?

The handbook explains the keyword as:

This keyword should only be applied by committers. The keyword signals that the ticket is a priority, and should be handled early in the next release cycle.

early means the commits need to happen during the early part of the Alpha cycle; else, the ticket gets bumped.

Though it would be great to prepare the work for commit as early as possible, commits for this ticket should not be constrained to only happen during the early Alpha phase. Rather, defect commits can happen up to 6.4 RC 1.

To aid in 6.4 tracking and scrubs, I've removed the early keyword.

I've also reset the keywords to reflect the ticket's current state.

#29 @swissspidy
21 months ago

  • Keywords fixed-major removed

Since this was already committed in 6.2 / 6.2.1, wouldn't it be better from a workflow perspective to open a new follow-up ticket? IIRC that's how we've done that in the past for such cases.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


21 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


20 months ago

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


20 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


20 months ago

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


20 months ago

#36 follow-up: @antpb
20 months ago

This looks to be fixed now. I'm going to dig into what changed but I have been able to verify that custom media library buttons and the featured image flow mentioned in #59308 are working

Here's a video showing newly uploaded files are rendering with the field correctly: https://streamable.com/3fieiz

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


20 months ago

#38 in reply to: ↑ 36 ; follow-up: @SeBsZ
20 months ago

Replying to antpb:

This looks to be fixed now. I'm going to dig into what changed but I have been able to verify that custom media library buttons and the featured image flow mentioned in #59308 are working

Here's a video showing newly uploaded files are rendering with the field correctly: https://streamable.com/3fieiz

@antpb, in which version of WordPress are you saying this has been fixed? Your video does not demonstrate that the issue in #59308 has been fixed as it is showing a fresh image upload. My issue only occurs when you click on an already attached featured image in combination with there needing to be many images in the media library such that the image you opened is not on the first page. If you want I can make a video that shows the bug in #59308?

#39 in reply to: ↑ 38 @antpb
20 months ago

hi @SeBsZ , a video would be very helpful if you could test on nightly. I was using trunk for my tests last night and found that this issue were in now 58051 is no longer reproducible. Could you try it for the featured image flows you mentioned in the other issue and leave a video there? That'll help me find a fix.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


20 months ago

#41 @nicolefurlan
19 months ago

Hi @SeBsZ! Could you take a look at #comment:39?

#42 @archon810
19 months ago

@nicolefurlan @SeBsZ already provided the screencast and a full repro, but in the appropriate ticket, which is https://core.trac.wordpress.org/ticket/59308#comment:3.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


19 months ago

#44 @oglekler
19 months ago

  • Milestone changed from 6.4 to 6.5

I used the code from https://core.trac.wordpress.org/ticket/59308#comment:3
and when I had few images uploaded this new field was appeared, but when I added more imaged to the Media library this bug started to appear.

We don't have patch yet and because we have 3 days before RC1, I am moving it into the next milestone.

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


15 months ago

#46 @rajinsharwar
15 months ago

  • Milestone changed from 6.5 to 6.6

There has not been much activity here since the last punt, punting it again for 6.6.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


14 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


12 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


11 months ago

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


11 months ago

#51 @sabernhardt
11 months ago

  • Milestone changed from 6.6 to 6.7

Please feel free to move this back to 6.6 if a patch is ready to commit before the end of the beta phase.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


9 months ago

#53 @joedolson
9 months ago

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

We're closing this ticket, as it's already a committed ticket. All continuing discussion should happen on the follow-up ticket, #59308.

#54 @sabernhardt
9 months ago

  • Keywords needs-patch removed
  • Milestone changed from 6.7 to 6.2.1
Note: See TracTickets for help on using tickets.