Skip to content

Conversation

@mzorz
Copy link
Contributor

@mzorz mzorz commented Feb 18, 2019

EDIT: updated description and title to reflect solution as added in 3e6608a

Fixes #9268 by making sure to only make host app -> editor calls to media upload progress reattachment code for Aztec, as Gutenberg relies on the RN side of things to request the host app to start sending updates the other way around (editor -> host app). This is done by means of the OnReattachQueryListener call once componenDidMount callback gets called on the JS side.

The crash started happening in current develop after the introduction of 0e2237f in #9030, after media upload progress reattachment code was merged.

There was a call in the SectionsPagerAdapter's instantiateItem when the Fragment was still not attached, but this was unneeded as Gutenberg works differently in this regard: instead of taking the list of currently progressing uploads and then telling the Editor to paint the progress when we know we're about to show it (i.e. onResume() or on the pager adapter instantiateItem call) as we do for Aztec, we need to wait to know the RN side of things is ready to receive progress updates, which happens after componentDidMount gets called, which in turn makes the query to the host app through the bridge's OnReattachQueryListener.

To test (copied from #9129):

  1. start a new draft
  2. insert an image
  3. once it starts uploading, exit the editor
  4. open the Post in the editor again
  5. observe the upload progress bar is updated correspondingly.

Try the same on Aztec posts as well.

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@mzorz mzorz added this to the 11.9 milestone Feb 18, 2019
@mzorz mzorz changed the title Call reattachUploadingMedia() only in onResume() Call reattachUploadingMedia() only for Aztec Feb 18, 2019
Copy link
Contributor

@marecar3 marecar3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@marecar3 marecar3 merged commit 7a5abb2 into develop Feb 18, 2019
@marecar3 marecar3 deleted the issue/9268-crash-image-upload-reattach-gb branch February 18, 2019 22:49
daniloercoli added a commit that referenced this pull request Feb 19, 2019
…ss-Android into gutenberg/remove-gb-warning-dialog

* 'develop' of https://github.com/wordpress-mobile/WordPress-Android: (63 commits)
  Re-replace private resources with custom ones
  Call reattachUploadingMedia() only for Aztec (#9269)
  Updating signature of function for Android 28
  Bump version to alpha-152/679
  Bump version to alpha-151/677
  Bump version to 11.8-rc-2/678
  Fix merge
  Introduce UNDEFINED_RES_ID and use it instead of the "-1" hard coded value
  Rename variable
  Update release notes
  Use the new specific page adapter for each use case
  Create separate adapters for search and page parent screen
  Rename PagesAdapter to PageListAdapter
  Fix androidTests
  Get site model from the bundle
  Gb/introduce media options sheet (#9251)
  Upgrade Tracks to 1.1.6
  Fix lint ignores for Gutenberg
  Update the gutenberg-mobile ref to the merged commit
  Minor code styling fix
  ...

# Conflicts:
#	WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefs.java
#	WordPress/src/main/res/values/strings.xml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants