wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.96k stars 1.31k forks source link

[GB-Mobile] Image upload doesn't work when don’t keep activities is ON or device is under heavy load #10163

Open daniloercoli opened 5 years ago

daniloercoli commented 5 years ago

Noticed that picture selection in the Image block doesn't work fine when don’t keep activities is ON, or when the device is under heavy load.

This is probably due to JS <----> Native bridge/listeners being corrupted when the image browser activity/screen is shown on the device and the host activity killed.

GB-mobile side Issue: https://github.com/wordpress-mobile/gutenberg-mobile/issues/582

develric commented 3 years ago

I spent a bit of time on this one; didn't come to a final conclusion since run out of time + hitted my limits with my current skills on RN 🙃 . Sharing anyway in case it can accelerate steps of anyone that will look later into it.

--- SAVE ---
           if (mEditorFragment != null) {
                      getSupportFragmentManager().putFragment(outState, STATE_KEY_EDITOR_FRAGMENT, mEditorFragment);
           }
--- RESTORE ---
           mEditorFragment =
                    (EditorFragmentAbstract) fragmentManager.getFragment(savedInstanceState, STATE_KEY_EDITOR_FRAGMENT);

            if (mEditorFragment instanceof EditorMediaUploadListener) {
                mEditorMediaUploadListener = (EditorMediaUploadListener) mEditorFragment;
            }

Used Pixel 3 Emu with API 29 and current WPAndroid develop (Vanilla 16.1-rc-1)

cameronvoell commented 3 years ago

dont keep activities on Start the wp-android app Tap on the button to add a new post Add an Image block Select a picture from the device Once the picture is selected you're back to the post with the Image block placeholder

This issue is still happening. More specifically this comment from the related gutenberg-mobile issue seems to be precisely what I am seeing: https://github.com/wordpress-mobile/gutenberg-mobile/issues/582#issuecomment-623595860 (Samsung Galaxy S10+, Android 11, WPAndroid 17.1 )

mchowning commented 2 years ago

I spent some time working on this, and got a partially working solution, but wasn't able to get something I felt should be merged before I had to move on to another task.

I think the core issue is that we use callbacks to update the javascript side of things with the selected media. When the editor activity is destroyed, those callbacks no longer work, and the relevant block doesn't get updated. Instead of using callbacks, we need to send events back to js with the selected image information and use those events to update the relevant block.

The difficulty with that approach is that the event is not inherently tied to a particular block like a callback is, so we need to include something in the event to identify the specific block to update and, currently, I don't think there is any information that is both persisted between activity restarts and can be used to reliably identify a given block.

The block's index in the post list gets close (that's what I used in my partially working solution), but it can fail in some cases (like long-running uploads) where the index of the block can change between the initiation of the image selection and its completion.

I think that the most promising solution is to persist the clientId for each block locally and to restore the clientId for each block from local storage when the editor is recreated. Then we could use the block's clientId to identify the relevant block. I did not have a chance to explore implementing this yet.

I wrote a slightly longer version of my thoughts on this here: p9ugOq-2HH-p2.