Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix null siteModel when EditorMedia.start() is invoked #10947

Merged
merged 3 commits into from Dec 17, 2019

Conversation

@malinajirka
Copy link
Contributor

malinajirka commented Dec 11, 2019

Fixes #10946

Review + merge instructions

  • wait until 13.8 gets merged into develop

Fixes two bugs

  • crash when EditPostActivity is started without a SiteModel.
  • EditorMedia.start is invoked before this and therefore is working with a wrong SiteModel.

I haven't been able to reproduce the issue. The fix is pretty simply, however, I'm not sure it's safe. Both bugs were introduced in 13.8-rc-1. Do you think it's worth merging into 13.8 - @planarvoid @jkmassel @mzorz ? => I've decided it's not worth the risk. The QuickPress widget won't work in this release.

To test:

  1. Add QuickPress widget to your desktop - link it to BlogA
  2. Go to the app and switch to BlogB
  3. Go to the desktop and click on the widget
  4. Notice the app doesn't crash

PR submission checklist:

  • I have considered adding unit tests where possible.

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile

This comment has been minimized.

Copy link

peril-wordpress-mobile bot commented Dec 11, 2019

You can test the changes on this Pull Request by downloading the APK here.

@malinajirka malinajirka modified the milestones: 13.8 ❄️, 13.9 Dec 12, 2019
@malinajirka malinajirka changed the base branch from release/13.8 to develop Dec 12, 2019
@malinajirka

This comment has been minimized.

Copy link
Contributor Author

malinajirka commented Dec 16, 2019

@planarvoid This is ready for review now. We are delaying the freeze, so it'd be great if you could review this first thing in the morning, thank you! 🙇

Copy link
Contributor

planarvoid left a comment

Looks good and as far as I can test it it works well 👍 :shipit:

@planarvoid planarvoid merged commit a286533 into develop Dec 17, 2019
6 checks passed
6 checks passed
Peril All green. Good on 'ya.
Details
ci/circleci: Installable Build Your tests passed on CircleCI!
Details
ci/circleci: connected-tests Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: strings-check Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
User Expectations [Android] automation moved this from Post Logic Improvements to Done Dec 17, 2019
@planarvoid planarvoid deleted the issue/10946-fix-npe branch Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.