Image/Media-Text: Low resolution image is uploaded if you close/re-open post during upload

[pinarol]

Detected during testing #1503.
Also reproducible @ 9f4afd6 develop
Tested with iPhone 8 plus, iOS 13.1

Reality

• Add media & text block, tap placeholder and choose Choose from device
• Start upload but before it finishes close the post by selecting Update Post
• Reopen the post and wait until the upload finishes
• Result: Low resolution pic is shown
• Open the WP Media library
• Result: Both low and high resolution version of the same image is uploaded

This is happening on Image block as well but it is less noticeable since the cover image has higher resolution.

Expected

• Add media & text block, tap placeholder and choose Choose from device
• Start upload but before it finishes close the post by selecting Update Post
• Reopen the post and wait until the upload finishes
• Verify that high resolution pic is shown
• Open the WP Media library
• Verify that only 1 image is uploaded

[koke]

Tested on WPAndroid develop with 1.15.1: I saw the red screen from #1509 but the uploaded image was full resolution, and there was only one item in the media library

[pinarol]

I'll add some root cause analysis I made about this. This is happening because there's a requestMediaImport call made on componentDidMount, and it is triggering a new upload for a local file (file:///). AFAIK this was done to be able to copy/paste images. But in our case it is pointing to the low res caption of uploaded image.

Actually for media-text we don't need to call requestMediaImport, we should be able to solve this just by removing it. However, we have the same problem on image block and I think this isn't noticed so far because the low res image is not low enough for image block. But it is still uploading both copies of the same image. So I was thinking maybe we should define a separate url in block attributes to store the url of the temporary image we are using during upload. This will introduce a difference between web & mobile but seems kinda normal if we think we do have different UX about media upload.

[pinarol]

BTW I remember trying to store the low res url in state rather than putting into attributes but that time it is reset if we close/reopen the post and so it didn't work.

[koke]

So I was thinking maybe we should define a separate url in block attributes to store the url of the temporary image we are using during upload

I was thinking maybe we could store that in state. I've been trying to understand the whole media upload process and I have some questions.

I understand it's valuable to store some temporary data like the upload ID and url so that the app can finish the upload async and replace the content even with the editor closed. But what happens if, let's say, the app crashes during upload? Would the existing attributes still be enough to match them with existing media and allow the user to retry the upload?

Could we do the same thing by relying only on mediaId and only changing the url when it's final? I guess that would need to improve the parsing beyond a regular search and replace like we discussed, but are there other things that I'm missing?

What I've seen so far (on the image block) is that the block attributes get updated a few times during upload (which matches what I noticed with the multiple undo steps):

1. We start with the placeholder
2. We receive a url with a low resolution thumbnail
3. We receive a url with a high resolution file
4. When the upload finishes, we receive the final url and id.

[pinarol]

Could we do the same thing by relying only on mediaId and only changing the url when it's final?

I tried that one: #1510 (comment) but state is getting reset when we close/re-open the post or so we won’t be able to show the low res image again during upload.

But what happens if, let's say, the app crashes during upload? Would the existing attributes still be enough to match them with existing media and allow the user to retry the upload?

It will match by using the upload mediaId if the main app is capable of continuing the upload. Let’s say the upload failed and the main app wasn’t able to tell the editor that it was failed, and the upload is just not continuing. Then the user needs to start over currently.

It should be possible to make use of requestMediaImport to start the upload but we need to be able to distinguish the low res url from the original local url.

[etoledom]

Fixed via WordPress/gutenberg#18215

(Automatic closing issues seems to not be working ¯\_(ツ)_/¯)