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/9748 npe in EditPostActivity onPrepareOptionsMenu #10534

Merged

Conversation

develric
Copy link
Contributor

Fixes #9748

The issue is confined to Android 8.0.0 and does not seems to be logic related.

It was not possible to reproduce the issue, but we decided to extend the defensive coding approach used in null check of all the other menu items in the onPrepareOptionsMenu also to the switchToAztecMenuItem and switchToGutenbergMenuItem. Note that the NPE happens on switchToAztecMenuItem that is the first not null checked item.

Notes

  • We added redundant checks on menu state coherence about switching between Aztec and Gutenberg editors in onOptionsItemSelected. This is a basic attempt to avoid odd conditions where a switch menu is available when it should not and you should not be allowed to switch between editors (AFAIU basically a post/page created in Aztec should not be allowed to be edited in GB; whereas a post/page created in GB can be edited with Aztec but the underline document will for now on contain blocks)
  • Only one switch editor menu at a time should be available always!
  • We would like to trace this event in Sentry. Currently I was only able to get events in Sentry as exception crashes. The real goal would be to trace events (not crashes) but could not find a way to trigger a normal event (possibly something to better investigate also with platform 9)
  • A possible improvement to this PR could be to notify the user with something like a toast or similar in case we are skipping to actuate the editor switch for inconsistent menu states.

Manual testing

The following test cases must be completed with and without the current PR patch and give the same results.

Test case 1 - Use block editor settings

  1. Go to My site > Settings and deactivate Use Block Editor
  2. Create a new Post/Page and check that no switch editor menu is available

Test case 2 - Use block editor settings

  1. Go to My site > Settings and activate Use Block Editor
  2. Create a new Post/Page and check that you are presented with the GB editor and you can switch between editors using menus (only one editor switch menu at a time should be available)

Test case 3 - From Aztec no GB

  1. Go to My site > Settings and ensure Use Block Editor is activated
  2. Create a new Post/Page and check that you are presented with the GB editor and switch to Aztec by menu
  3. Edit the document with Aztec and Save it
  4. open again the document and check that you do not have the switch to GB menu available

Test case 4 - From GB to Aztec and return

  1. Go to My site > Settings and ensure Use Block Editor is activated
  2. Create a new Post/Page and check that you are presented with the GB editor
  3. Edit the document with GB and Save it
  4. open again the document and check that you get GB editor and you can freely switch and edit to Aztec and again to GB

Test case 5 - General smoke test on Editor and Menu items

  1. Create documents with both editors and reopen them passing from one Editor to another where allowed.
  2. Use all menus available trying to cover the various states

Update release notes:

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

@peril-wordpress-mobile
Copy link

Warnings
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

@peril-wordpress-mobile
Copy link

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

@develric develric marked this pull request as ready for review September 27, 2019 03:01
@develric develric requested a review from planarvoid September 27, 2019 03:02
Copy link
Contributor

@planarvoid planarvoid left a comment

Choose a reason for hiding this comment

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

as far as I can see and test this, it seems to work well :-) good job

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash: NPE in EditPostActivity OnPrepareOptionsMenu
2 participants