Deleting an image via the media library is not reflected.

[Fujix1]

Deleting an image permanently on the media library is not reflected to the related image block.

Step to reproduce

1. Click the "Replace" menu item to open the media library.
2. Delete the image permanently.
3. Click the X button to close the media libarary.
4. The image is still displayed in the image block.
5. Actually the image is not deleted and the frontend still has it.

Expected behavior
The image should be removed from the image block in sync with the media library, and the image itself is to be deleted from the server.

Screenshots

Editor version (please complete the following information):

• WordPress version: 5.4.2
• Does the website has Gutenberg plugin installed, or is it using the block editor that comes by default? Both.
• If the Gutenberg plugin is installed, which version is it? v8.3.0

Desktop (please complete the following information):

• OS: Windows 10
• Browser: Chrome
• Version: 83.0.4103.106

[annezazu]

@Fujix1 Thanks for reporting this! I was able to replicate this in Gutenberg 8.4.0 as well and have labeled this as a bug.

[ramonjd]

I spent some time looking at the Image Block. I didn't find a suitable fix yet, but here's what I learned in case someone else gets to this sooner:

The Image Block attribute values id and url persist in the Block Editor, even after the image has been deleted in the media library.

getMedia( id ) seems to be an accurate guide as to whether the Image Block attachment file is available or not, so we could check this in the useSelect in edit.js.

In order to "force" the MediaPlaceholder to show when getMedia( id ) returns undefined, we'll also have to ensure the incoming props – id, mediaPreview and disableMediaButtons – don't contain any cached image data.

			<MediaPlaceholder
				icon={ <BlockIcon icon={ icon } /> }
				onSelect={ onSelectImage }
				onSelectURL={ onSelectURL }
				notices={ noticeUI }
				onError={ onUploadError }
				accept="image/*"
				allowedTypes={ ALLOWED_MEDIA_TYPES }
				value={ { id: isImageDataAvailable ? id : undefined, src } }
				mediaPreview={ isImageDataAvailable ? mediaPreview : undefined }
				disableMediaButtons={ isImageDataAvailable ? ( temporaryURL || url ) : undefined }
			/>

The challenge I faced when testing was finding a source of truth. It'd be great to call setAttributes( { id: undefined, url: undefined } ) when getMedia( id ) returns undefined, but since it's an async call we'd have to wait a couple of cycles to get our answer.

This means there might be a bit of shuffling around required, for example, rather than relying on the image block attributes for id and url values in edit.js, we wait and get them from the getMedia( id ). Maybe...

All this seems a bit complex, so I might be missing a more straightforward way of going about things, for instance, a server-side method or something else.

[ramonjd]

Adding some notes after chatting with @andrewserong, who has helped with the following advice:

The hard part is that the getMedia approach might work on a fresh page load, but in the reported example, it sounds like we’d want it to respond immediately (or thereabouts) based on a removal from the media library, without a hard refresh…

There’s then the consideration of a large post containing dozens or more images in a photo gallery. We wouldn’t want to accidentally cause double the HTTP requests to render the editor view just to check whether or not to render the placeholder

I suppose the ideal thing would be if we can observe / fire off an action somehow (!?) to perform the check synchronously for one image on a delete event instead of having to proactively check for the existence of media on every image?

We might be able to utilize the delete_attachment action hook (?)

Also wp.media.attachment( id ) doesn’t seem to make any Ajax calls, and appears to be a fairly reliable check. It might be worth investigating this further.

[ramonjd]

I spent some more time looking at this issue, a little deflated after trying to wade through the core media js library, and didn't get so far from a technical implementation perspective, but I have questions that might lead us in a productive direction.

Within a Gutenberg component, such as image/edit.js, we can in fact check the media Attachment collection. For example, something like this, while not pretty and perfect, works to a certain extent:

useEffect( () => {
	if ( ! isSelected ) {
		const mediaAttachment = **wp**?.media?.attachment( id );
		if (
			mediaAttachment &&
			mediaAttachment?.destroyed === true
		) {
			setAttributes( {
				url: undefined,
				alt: undefined,
				id: undefined,
				title: undefined,
				caption: undefined,
			} );
			setTemporaryURL( undefined );
		}
	}
}, [ isSelected ] );

However I think a better option might be to have core media library trigger an event in the attachment model, perhaps in the callback of the delete HTTP request.

"Better" because consumers of <MediaUpload /> in Gutenberg – assuming <MediaUpload /> subscribes to any delete attachment event – could pass, for example, an onDeleteAttachment callback, and then decide to do something based on the deleted attachment id.

I'm not very familiar with Backbone.js so perhaps listening to the remove event might be better, I'm not sure. Also, there might very well be an event that we can listen to already :)

Out of interest, here's where the UI event to delete the attachment is set: https://github.com/wordpress/wordpress-develop/blob/9bac3d3401554592d424c47ab2f0b35b309f1293/src/js/media/views/attachment/details.js#L151

Imagining we've got that far, we'd then have to allow the the Gutenberg MediaUpload utils to listen out for a delete attachment event, either on the collection or the selected attachment.

I'm not sure how to do this quite yet. In Gutenberg, we listen to this.frame.on( 'someEvent'), so I'm guessing we'd have to listen out for changes to the Attachment model in the Frame view and trigger an action there.

[andrewserong]

Thanks for digging in further to this one @ramonjd!

I think you've already covered this, but If we can, it'd be great to try to keep any of the media library logic tucked inside the media upload component in the media-utils package so that it's encapsulated. This should help ensure that the event handling / wp.media behaviour is only executed in environments where the core media library is available. That could be tricky, though! It looks like there's a couple of places outside of this package that reference wp?.media (E.g. wp?.media?.view?.settings?.defaultProps?.link in the image block and the gallery block), but I imagine we're trying to limit that kind of usage.

Imagining we've got that far, we'd then have to allow the the Gutenberg MediaUpload utils to listen out for a delete attachment event, either on the collection or the selected attachment.

I'm not sure how to do this quite yet. In Gutenberg, we listen to this.frame.on( 'someEvent'), so I'm guessing we'd have to listen out for changes to the Attachment model in the Frame view and trigger an action there.

This sounds like a good approach to explore. I'm not quite sure how the Backbone code works, but in the Attachment model's sync function, when the attachment is deleted, its destroyed attribute is set to true. So it sounds like if we can set up a listener on the Attachment model, if we can watch for this change, that might be a way to hook into it from the MediaUpload component.

From a quick read of the Backbone JS docs on collections it sounds like there are some inbuilt events whenever changes occur on a model (under the Catalog of Events heading further down the page). So 🤞 we might be able to listed for update, change, or possibly even change:destroyed if the change in attribute is automatically recognised by Backbone?

[adamsilverstein]

@ramonjd - wp.media will trigger an event when an item is removed from the library, you can listed for something like wp.customize.control.on( 'removed', ( item ) => { ...handler } ) where item is the removed item.

[ramonjd]

@adamsilverstein 🙇 Thank you so much! I'll try it out.

Edit: this also works:

this.frame.listenTo(
	wp.media.model.Attachments.all,
	'remove',
	( attachmentObject ) => attachmentObject.destroyed ? console.info( 'It be gawn, dawg!' ) : null;
);
 

[ramonjd]

#35973 had to be reverted for WordPress 6.0 due to the following issues: #41161

Reopening. New WIP PR:

• Image: Reflect media deletion in the editor #41220

[Mamaduka]

I think we can go with the solution described here but make it non-destructive. If an image is missing, the block doesn't reset its attributes; instead, we display MediaPlaceholder with some info. cc @jasmussen

Some technical notes:

• We should only do this check when media is hosted on the same WP installations - the block has the media ID.
• For performance reasons, the getMedia for the image block only runs when selected. We could probably use the hasImageErrored state to force-run it.
• We could use getResolutionError to ensure that the returned value is undefined because of missing media and not for any other reason. The error will return rest_post_invalid_id code.

[jasmussen]

we display MediaPlaceholder with some info

This seems okay if we can think of an approach that is truly generic across any content that drifts out of sync. Coming to mind is not just images, but also videos, files, sound bits, and even navigation blocks where the nav CPT itself gets deleted. Conceptually those are all the same: the markup refers to something that's not there anymore.

The one take-away I have, to avoid #64288 again, is that the placeholder pattern is not a good place to show error messages. To that end, my instinct would be to go towards what the web-browser we all use has established as a pattern for similar cases: showing a broken symbol. We can do that, and better, by showing error information in the inspector, where there's room to both be verbose, and provide actions to help address the situation.

Separately, and this should be tracked somewhere, on my mind is a "linting" panel, perhaps it exists in the document outline, or in the prepublish flow, or both, that exists to surface warnings and errors for your document, just like an IDE does. Anything from broken image to insufficient contrast, poor SEO or missing API key.

[Mamaduka]

To that end, my instinct would be to go towards what the web-browser we all use has established as a pattern for similar cases: showing a broken symbol. We can do that, and better, by showing error information in the inspector, where there's room to both be verbose, and provide actions to help address the situation.

Exactly! A while ago, I read an article about styling broken images via CSS, which took a similar approach. I think we need a design for a similar component - BrokenMedia or MissingMedia, something generic that can be used across the blocks.

We could also provide error info via this component since it would be the "same" for all the core blocks - the entity for the provided ID is missing from the server.

[jasmussen]

If we start with the image, then some kind contributor has designed this in the Icon library:

That's this SVG:

<svg width="24" height="24" fill="none" xmlns="http://www.w3.org/2000/svg"><path fill-rule="evenodd" clip-rule="evenodd" d="M5 3a2 2 0 0 0-2 2v14a2 2 0 0 0 2 2h5l-.121-.121 1.379-1.379H5a.5.5 0 0 1-.5-.5v-2.436l4.096-2.987 2.996 1.941a.75.75 0 0 0 .93-.091L16 12.046l1.375 1.337 1.06-1.06-1.912-1.86a.75.75 0 0 0-1.046 0l-3.571 3.471-2.927-1.897a.75.75 0 0 0-.85.024L4.5 14.707V5a.5.5 0 0 1 .5-.5h14a.5.5 0 0 1 .5.5v6.258l1.5-1.5V5a2 2 0 0 0-2-2H5Zm16 11-1.5 1.5V19a.5.5 0 0 1-.5.5h-3.5L14 21h5a2 2 0 0 0 2-2v-5Z" fill="#1E1E1E"/></svg>

My instinct would go towards using that:

• On the same gray background as placeholder imagery.
• If the block has dimensions set already (sounds like it has if it's markup with drift), keep those dimensions intact.
• If those dimensions are less than 24x24, then allow the icon to be cropped, just like how the browser would crop it.

If we choose to expand to additional broken filetypes, we can either use the same pattern to make broken versions of video, file, etc, or make a generic broken item icon we can use. But if you agree, it may be fine to start here.

[ramonjd]

Thank you for reviving this issue @Mamaduka and @jasmussen

I'm not sure when I'll get a chance to return to it, but it'sgreat to have these notes.