Retry to download an image if a download error occured after an upload

[maxme]

Retry to download an image if a download error occured after an upload - workaround to fix wordpress-mobile/WordPress-Editor-Android#300

Previous onError implementation seemed weird (imageNode.attr('src', image.src);), not sure if it was working. I tried to upload images on private sites, and everything works as expected.

[maxme]

Note: it's a pure WordPress-Editor-Android patch, but the issue is reproducible with the wpandroid app only.

[aforcier]

This was needed to make sure the image inside the container (the 'real' image, not the one we load into memory using the image object) is set to the remote URL. With this line removed, if the image fails to load, the local image isn't swapped out, resulting in a 'loaded'-looking image with a local (Android filesystem) src, which can be uploaded.

We should keep this, so that even if the image fails and never loads for some reason, the post is still uploaded with a remote URL. The comment doesn't make its use clear though, I think we can add some clarification.

[aforcier]

Follow-up note: this is a little trickier than I initially realized, since we'll be showing the user a dead image icon for however long it takes to load the image from the server. This might only be for 500ms, but it's still not ideal. I still think that's better than risking uploading the post with the (wrong) local image url.

A permanent solution might be to keep the line removed as you had it, but make sure the local src is swapped for the remote one if the user attempts to upload before the image has been loaded from the server from onerror retries.

[maxme]

This might only be for 500ms, but it's still not ideal. I still think that's better than risking uploading the post with the (wrong) local image url.

Yes, it's a bit weird, but I think it's better to not change the HTML source to fix a graphic glitch - we could end up breaking the post.

Here a demo with a 5 secs delay:
https://cloudup.com/cErFyGdP1GX

A permanent solution might be to keep the line removed as you had it, but make sure the local src is swapped for the remote one if the user attempts to upload before the image has been loaded from the server from onerror retries.

What would be the best solution to mark it "unswapped yet", add a class to the img?

[aforcier]

What would be the best solution to mark it "unswapped yet", add a class to the img?

I think a good solution would be adding a remoteUrl attribute, set to the remote URL once we have it. Then we add an extra step to ZSSEditor.removeVisualFormatting() to find any image tags with that attribute and swap out the src for remoteUrl. That should cover the case where the image never loads before publishing.

[aforcier]

From testing with a .org site (adding a character in the code when setting the image src originally to force a 404, then renaming the image server-side while the editor attempts to reload), the retries after the initial attempt load from the cache instead of attempting a fresh network call, so that the image is never loaded even when it's available on the server.

One fix would be to append a '?retry=' + nCall string when re-setting the node.src value in ZSSEditor.reloadImage. From my testing this correctly forces a network reload each time, updating the image as soon as I fix the src server-side. We'd have to make sure this doesn't interfere at all with Photon url params though (haven't tested that).

[maxme]

From testing with a .org site (adding a character in the code when setting the image src originally to force a 404, then renaming the image server-side while the editor attempts to reload), the retries after the initial attempt load from the cache instead of attempting a fresh network call, so that the image is never loaded even when it's available on the server.

Never happened during my tests, seems weird, the 404 error call shouldn't be cached.

[maxme]

Cache-Control on 404 errors make sense in some cases, so we want to use the ?retry trick. I'll also work on adding a temporary remoteUrl attribute since we'll have to clean the image tag in ZSSEditor.removeVisualFormatting() anyway.

[maxme]

Added a remoteurl attribute in 7fba5d6 - so the user won't see a broken image in case of a temporary 404, he'll see the "Uploading..." message until he gets a working image or a pseudo-timeout (7 retries).

[maxme]

Closing this and deleting the branch - followup in #3896