Gallery block: Some images point to local URLs when closing the post with ongoing uploads #33593

fluiddot · 2021-07-21T11:04:44Z

Description

Some images point to local URLs (example: //private/var/mobile/Containers/Data/Application/...) when closing the post with ongoing uploads and re-opening when they're finished.

Step-by-step reproduction instructions

This can be reproduced by following the "Close post with an ongoing image upload" test case:

Add a gallery block and tap "Add Media"
Select "Choose from device" option
Select multiple images from the device and confirm the selection
While image are uploading, leave the editor
Wait for uploads to complete while in the post list
Re-open the post with the gallery block

Expected behaviour

The images' URL scheme should be https://.

Actual behaviour

The images' URL scheme is ///.

Screenshots or screen recording (optional)

Uploading images	Last frame before closing the post	Re-open post	HTML mode

NOTE: This issue might be related to closing the post when the image is already uploaded but not loaded in the editor, as you can see in the screenshots, the first four items are showing a placeholder image right before the post is closed, and coincidentally these items are the ones that point to local URLs.

WordPress information

WordPress version: N/A
Gutenberg version: N/A
Are all plugins except Gutenberg deactivated? N/A
Are you using a default theme (e.g. Twenty Twenty-One)? N/A

Device information

Device: iPhone 11
Operating system: iOS 14.2
WordPress app version: develop

The text was updated successfully, but these errors were encountered:

fluiddot · 2021-07-21T13:48:17Z

I managed to reproduce this issue on Android too by adding a big batch of images and re-opening the post while the uploads are ongoing. It's harder to reproduce but I saw some images pointing to file://storage/emulated/....

mchowning · 2021-07-21T14:33:16Z

Thanks for the detailed report @fluiddot !

I managed to reproduce this issue on Android too by adding a big batch of images and re-opening the post while the uploads are ongoing. It's harder to reproduce but I saw some images pointing to file://storage/emulated/....

👍 This sounds like wordpress-mobile/gutenberg-mobile#1526.

fluiddot · 2021-07-21T14:35:42Z

Thanks for the detailed report @fluiddot !

I managed to reproduce this issue on Android too by adding a big batch of images and re-opening the post while the uploads are ongoing. It's harder to reproduce but I saw some images pointing to file://storage/emulated/....

👍 This sounds like wordpress-mobile/gutenberg-mobile#1526.

Right, as per that issue description, it looks like it's the same issue.

fluiddot · 2021-09-14T17:38:15Z

NOTE: This issue might be related to closing the post when the image is already uploaded but not loaded in the editor, as you can see in the screenshots, the first four items are showing a placeholder image right before the post is closed, and coincidentally these items are the ones that point to local URLs.

This issue is in fact related to a race condition between closing the post and images already uploaded, here is an example of a test I made that proves it:

Preparation:

Add the following code print("Media processed: \(media)") to WordPress-iOS/WordPress/Classes/Services/PostCoordinator.swift#L332. This will help us identify which images are being processed (replace the local file URL with remote one) in the native side after they're uploaded.
Add the following code to packages/block-library/src/gallery/v1/gallery-image.native.js#L156. This will help us to identify which images are uploaded and are notified to the Editor.

console.log( 'finished', {
    id: payload.mediaServerId,
    url: payload.mediaUrl,
} );

gutenberg/packages/block-library/src/gallery/v1/gallery-image.native.js

Line 156 in 8714f66

finishMediaUploadWithSuccess( payload ) {

Add the following code console.log( 'serializeToNativeAction', html ); to packages/editor/src/components/provider/index.native.js#L289. This will help us to identify the last time the Editor serializes the post's content and sends it to the native side before saving it.

gutenberg/packages/editor/src/components/provider/index.native.js

Line 289 in 8714f66

RNReactNativeGutenbergBridge.provideToNative_Html(

Steps

The steps followed in the tests are:

Since we need to check the console traces, build the app in Xcode and run it with the Metro server connected.
Open a post.
Add a Gallery block.
Insert 6 images from the device.
Right after inserting the images, close the post and save it.
Check Xcode and RN consoles.

Results

RN console:

Xcode console

Media processed: data: {
    ...
    filename = "img_0005-3-2.jpg";
    filesize = 1842362;
    height = 2002;
    length = nil;
    localThumbnailIdentifier = p156;
    localThumbnailURL = "thumbnail-p156-2778x2778.jpeg";
    localURL = "img_0005-3.jpeg";
    mediaID = 1586;
    mediaTypeString = image;
    ...
})
Media processed: data: {
    ...
    filename = "img_0111-3-2.jpg";
    filesize = 5045663;
    height = 3024;
    length = nil;
    localThumbnailIdentifier = p155;
    localThumbnailURL = "thumbnail-p155-2778x2778.jpeg";
    localURL = "img_0111-3.jpeg";
    mediaID = 1587;
    mediaTypeString = image;
    ...
})
Media processed: data: {
    ...
    filename = "img_0001-3-2.jpg";
    filesize = 2221060;
    height = 2848;
    length = nil;
    localThumbnailIdentifier = p160;
    localThumbnailURL = "thumbnail-p160-2778x2778.jpeg";
    localURL = "img_0001-3.jpeg";
    mediaID = 1588;
    mediaTypeString = image;
    ...
})
Media processed: data: {
    ...
    filename = "img_0003-3-2.jpg";
    filesize = 2479681;
    height = 2002;
    length = nil;
    localThumbnailIdentifier = p158;
    localThumbnailURL = "thumbnail-p158-2778x2778.jpeg";
    localURL = "img_0003-3.jpeg";
    mediaID = 1589;
    mediaTypeString = image;
    ...
})
Media processed: data: {
    ...
    filename = "img_0002-3-2.jpg";
    filesize = 2999217;
    height = 2848;
    length = nil;
    localThumbnailIdentifier = p159;
    localThumbnailURL = "thumbnail-p159-2778x2778.jpeg";
    localURL = "img_0002-3.jpeg";
    mediaID = 1590;
    mediaTypeString = image;
    ...
})

Post HTML

<!-- wp:gallery {"ids":[1587,1586,1753731616,1589,1590,1588],"linkTo":"none"} -->
<figure class="wp-block-gallery columns-3 is-cropped">
    <ul class="blocks-gallery-grid">
        <li class="blocks-gallery-item">
            <figure>
                <img src="https://<SITE>.files.wordpress.com/2021/09/img_0111-3-2.jpg" data-id="1587" class="wp-image-1587"/>
            </figure>
        </li>
        <li class="blocks-gallery-item">
            <figure>
                <img src="https://<SITE>.files.wordpress.com/2021/09/img_0005-3-2.jpg" data-id="1586" class="wp-image-1586"/>
            </figure>
        </li>
        <li class="blocks-gallery-item">
            <figure>
                <img src="///<LOCAL_URL>/Containers/Data/Application/<APP_ID>/tmp/1753731616.jpg" data-id="1753731616" class="wp-image-1753731616"/>
            </figure>
        </li>
        <li class="blocks-gallery-item">
            <figure>
                <img src="https://<SITE>.files.wordpress.com/2021/09/img_0003-3-2.jpg" data-id="1589" class="wp-image-1589"/>
            </figure>
        </li>
        <li class="blocks-gallery-item">
            <figure>
                <img src="https://<SITE>.files.wordpress.com/2021/09/img_0002-3-2.jpg" data-id="1590" class="wp-image-1590"/>
            </figure>
        </li>
        <li class="blocks-gallery-item">
            <figure>
                <img src="https://<SITE>.files.wordpress.com/2021/09/img_0001-3-2.jpg" data-id="1588" class="wp-image-1588"/>
            </figure>
        </li>
    </ul>
</figure>
<!-- /wp:gallery -->

Image upload status:

1585 🔴: Uploaded while the editor was opened, it was notified to RN but the URL attribute was updated after the HTML was serialized.
1586 ✅: Uploaded while the editor was opened, it was notified to RN but the URL attribute was updated after the HTML was serialized. However, the image was processed by the native side and its URL was edited.
1587 ✅: Uploaded while the editor was opened, it was notified to RN but the URL attribute was updated after the HTML was serialized. However, the image was processed by the native side and its URL was edited.
1588 ✅: Uploaded after the editor was closed.
1589 ✅: Uploaded after the editor was closed.
1590 ✅: Uploaded after the editor was closed.

As you can see the images were uploaded but one is pointing to a local URL (1585) in the HTML, this happened due to the fact that the post HTML was serialized before the URL attribute of that image was updated in the editor. Besides, the image was considered already uploaded in the native side and therefore wasn't either processed which would have caused the URL to be updated after the post was closed.

Potential workarounds

Serialize the post's content after all RN bridge calls are finished
NOTE: This option would be the optimal one as we would assure that the post HTML reflects the most recent snapshot of the content. Nevertheless, I think this can be tricky to achieve because it could imply refactoring the structure of the RN bridge. Another alternative would be to trigger the serializing function when the root-level Editor component is unmounted.
Detect local image URLs and replace them with the remote version

jd-alexander · 2021-09-14T22:18:30Z

Good investigation here @fluiddot I want to add some context here that is related to the point below.

Detect local image URLs and replace them with the remote version

When working on the Audio block we had to create processors in both platforms that detected the local URLs and replaced them with the remote versions.

wordpress-mobile/WordPress-Android#13575
wordpress-mobile/WordPress-iOS#15420

Here are all the processors replacing local media URLs with remote versions on Android
The linked PostCoordinator.swift you have above contains references to the iOS processors.

I am guessing based on your explanation above we may need to do some optimizations to the serialization process or optimize the processors to support this case.

I managed to reproduce this issue on Android too by adding a big batch of images and re-opening the post while the uploads are ongoing. It's harder to reproduce but I saw some images pointing to file://storage/emulated/....

Does this mean sometimes the local URLs are replaced and other times they are not? I am guessing the latter is the case.

This issue is in fact related to a race condition between closing the post and images already uploaded, here is an example of a test I made that proves it:

Ah, this is quite interesting!

fluiddot · 2021-09-15T09:04:37Z

When working on the Audio block we had to create processors in both platforms that detected the local URLs and replaced them with the remote versions.

wordpress-mobile/WordPress-Android#13575
wordpress-mobile/WordPress-iOS#15420

Here are all the processors replacing local media URLs with remote versions on Android

The linked PostCoordinator.swift you have above contains references to the iOS processors.

I am guessing based on your explanation above we may need to do some optimizations to the serialization process or optimize the processors to support this case.

Thanks @jd-alexander for the info 🙇 , I learned about the existence of the processors while testing, I haven't explored further how they work but I have a rough idea. As far as I investigated, the media that ends up with the local URL is not actually processed because technically it's already uploaded (see the explanation for this below).

Does this mean sometimes the local URLs are replaced and other times they are not? I am guessing the latter is the case.

Not exactly, all media items that are being uploaded or pending upload are properly processed and their local URLs are replaced. However, there's a case where some media items have just finished the upload while the post is being closed, which would imply that they won't be processed on the native side because they're considered already uploaded. In theory, since the upload is completed the URL should be correct but unfortunately, it's not because the editor serialized the HTML that will be saved before the upload complete events are received on the RN side.

fluiddot · 2021-09-15T10:06:56Z

Does this mean sometimes the local URLs are replaced and other times they are not? I am guessing the latter is the case.

Not exactly, all media items that are being uploaded or pending upload are properly processed and their local URLs are replaced. However, there's a case where some media items have just finished the upload while the post is being closed, which would imply that they won't be processed on the native side because they're considered already uploaded. In theory, since the upload is completed the URL should be correct but unfortunately, it's not because the editor serialized the HTML that will be saved before the upload complete events are received on the RN side.

On iOS, I noticed that in case there are no media items being uploaded when saving a post, it actually updates the URL because it processes all current media items of the post 🤔 .

iOS reference

So a potential solution would be to process completed media items when the post is uploading media, or process all items once the uploads have finished.

On Android looks like it always processes all media items, here I listed the methods involved in processing the post after being saved:

doFinalProcessingOfPosts
hasPendingOrInProgressMediaUploadsForPost
updateOnePostModelWithCompletedAndFailedUploads
updatePostWithCurrentlyCompletedUploads
For all completed media of the post: updatePostWithMediaUrl

I'll debug this case on Android in case the cause is different from iOS 🕵️ .

fluiddot · 2021-09-15T11:23:24Z

On Android looks like it always processes all media items, here I listed the methods involved in processing the post after being saved:

doFinalProcessingOfPosts

hasPendingOrInProgressMediaUploadsForPost

updateOnePostModelWithCompletedAndFailedUploads

updatePostWithCurrentlyCompletedUploads

For all completed media of the post: updatePostWithMediaUrl

replaceMediaFileWithUrlInPost

replaceMediaFileWithUrlInGutenbergPost

processContent

I'll debug this case on Android in case the cause is different from iOS 🕵️ .

I couldn't manage to reproduce the issue by following the same repro steps I used for iOS, however, I encountered local URLs by following the flow I commented on some time ago:

I managed to reproduce this issue on Android too by adding a big batch of images and re-opening the post while the uploads are ongoing. It's harder to reproduce but I saw some images pointing to file://storage/emulated/....

Steps:

Open a post.
Add a Gallery block.
Insert 6 images from the device (we could insert more, the idea here is to have enough time to save the post and open it again).
Right after inserting the images, close the post and save it.
Open the post again.
Observe that some images have local URLs.

EDIT: I tested this on the latest production version (18.1) and it's quite difficult to reproduce, I only managed to do it once following the exact same steps, so probably for Android we could consider this issue not a high priority.

fluiddot · 2021-09-15T15:28:31Z

The fix for iOS will be introduced in this PR 🎊 . I'd appreciate it if an iOS developer could take a look, @ceyhun could you spare some time for this? Thanks 🙇 !

ceyhun · 2021-09-16T13:53:33Z

Hey @fluiddot 👋 I tried this quite a few times with different combinations but I couldn't reproduce. I tried on WPiOS's develop branch (wordpress-mobile/WordPress-iOS@c67e4ad) and also on the WPiOS PR's branch (wordpress-mobile/WordPress-iOS#17157) by commenting out the code in PostCoordinator.swift, .
I tried both with and without metro. I also tried with simulator and a real device. Here's a recording of how it looks for me on iPhone 11 Pro, iOS 14.8, with metro connected, on free WP.com site. Let me know if I missed a step or if I should try with another combination.

gallery-bug.MP4

fluiddot · 2021-09-17T09:53:18Z

Thanks @ceyhun for reviewing the PR ❤️ !

Hey @fluiddot 👋 I tried this quite a few times with different combinations but I couldn't reproduce. I tried on WPiOS's develop branch (wordpress-mobile/WordPress-iOS@c67e4ad) and also on the WPiOS PR's branch (wordpress-mobile/WordPress-iOS#17157) by commenting out the code in PostCoordinator.swift, .
I tried both with and without metro. I also tried with simulator and a real device. Here's a recording of how it looks for me on iPhone 11 Pro, iOS 14.8, with metro connected, on free WP.com site. Let me know if I missed a step or if I should try with another combination.

It can be a bit tricky to reproduce it because you have to close the post right after any of the uploads just finished. After watching the video you posted, I think you closed the post a bit sooner before an upload finishes to trigger the issue, if you waited a bit more I think you'll bump into it.

I've tested it locally in a simulator with Metro connected and I managed to reproduce it, let me capture the steps I followed in the following video to help you reproducing it:

Kapture.2021-09-17.at.11.39.23.mp4

In the following screenshots, you can see the exact frames where I update the post and the editor closes, notice that two of the images are showing a placeholder at that moment that matches the image tags that include local URLs:

Updating draft	Editor closing

Let me know if you manage to reproduce it and if not, I'll be more than happy to set up a quick call to check it on live.

ceyhun · 2021-09-20T11:52:02Z

Thanks @fluiddot 🙇 I was able to reproduce following these instructions 👍

fluiddot added Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Block] Gallery Affects the Gallery Block - used to display groups of images labels Jul 21, 2021

fluiddot mentioned this issue Jul 21, 2021

[TEST][DO NOT MERGE] Smoke test - Gutenberg Mobile 1.58.0 alpha version wordpress-mobile/WordPress-iOS#16892

Closed

4 tasks

fluiddot changed the title ~~[Only iOS] Gallery block: Some images point to local URLs when closing the post with ongoing uploads~~ Gallery block: Some images point to local URLs when closing the post with ongoing uploads Jul 21, 2021

fluiddot mentioned this issue Jul 21, 2021

[TEST][DO NOT MERGE] Smoke test - Gutenberg Mobile 1.58.0 alpha version wordpress-mobile/WordPress-Android#15046

Closed

3 tasks

mchowning added the [Priority] High Used to indicate top priority items that need quick attention label Jul 21, 2021

fluiddot self-assigned this Sep 13, 2021

fluiddot mentioned this issue Sep 15, 2021

Update synced media references when saving a post with ongoing uploads wordpress-mobile/WordPress-iOS#17157

Merged

4 tasks

fluiddot closed this as completed in wordpress-mobile/WordPress-iOS#17157 Sep 20, 2021

ParaskP7 mentioned this issue Jul 7, 2022

[Fix] Remove Only the Default Initializer to Provide Custom Configuration wordpress-mobile/WordPress-Android#16881

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gallery block: Some images point to local URLs when closing the post with ongoing uploads #33593

Gallery block: Some images point to local URLs when closing the post with ongoing uploads #33593

fluiddot commented Jul 21, 2021 •

edited

Loading

fluiddot commented Jul 21, 2021

mchowning commented Jul 21, 2021

fluiddot commented Jul 21, 2021

fluiddot commented Sep 14, 2021

jd-alexander commented Sep 14, 2021

fluiddot commented Sep 15, 2021 •

edited

Loading

fluiddot commented Sep 15, 2021

fluiddot commented Sep 15, 2021 •

edited

Loading

fluiddot commented Sep 15, 2021

ceyhun commented Sep 16, 2021

fluiddot commented Sep 17, 2021 •

edited

Loading

ceyhun commented Sep 20, 2021

Gallery block: Some images point to local URLs when closing the post with ongoing uploads #33593

Gallery block: Some images point to local URLs when closing the post with ongoing uploads #33593

Comments

fluiddot commented Jul 21, 2021 • edited Loading

Description

Step-by-step reproduction instructions

Expected behaviour

Actual behaviour

Screenshots or screen recording (optional)

WordPress information

Device information

fluiddot commented Jul 21, 2021

mchowning commented Jul 21, 2021

fluiddot commented Jul 21, 2021

fluiddot commented Sep 14, 2021

Preparation:

Steps

Results

Potential workarounds

jd-alexander commented Sep 14, 2021

fluiddot commented Sep 15, 2021 • edited Loading

fluiddot commented Sep 15, 2021

fluiddot commented Sep 15, 2021 • edited Loading

fluiddot commented Sep 15, 2021

ceyhun commented Sep 16, 2021

fluiddot commented Sep 17, 2021 • edited Loading

ceyhun commented Sep 20, 2021

fluiddot commented Jul 21, 2021 •

edited

Loading

fluiddot commented Sep 15, 2021 •

edited

Loading

fluiddot commented Sep 15, 2021 •

edited

Loading

fluiddot commented Sep 17, 2021 •

edited

Loading