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

Crash: NPE in EditPostActivity OnPrepareOptionsMenu #9748

Closed
loremattei opened this issue Apr 29, 2019 · 12 comments · Fixed by #10534
Closed

Crash: NPE in EditPostActivity OnPrepareOptionsMenu #9748

loremattei opened this issue Apr 29, 2019 · 12 comments · Fixed by #10534

Comments

@loremattei
Copy link
Contributor

This seems to be new in 12.1.1 and it has been very common in 12.1.2 and 12.2.

Fatal Exception: java.lang.NullPointerException: Attempt to invoke interface method 'android.view.MenuItem android.view.MenuItem.setVisible(boolean)' on a null object reference
       at org.wordpress.android.ui.posts.EditPostActivity.onPrepareOptionsMenu(EditPostActivity.java:1188)
       at android.app.Activity.onPreparePanel(Activity.java:3483)
       at android.support.v4.app.FragmentActivity.onPrepareOptionsPanel(FragmentActivity.java:559)
       at android.support.v4.app.FragmentActivity.onPreparePanel(FragmentActivity.java:547)
       at android.support.v7.view.WindowCallbackWrapper.onPreparePanel(WindowCallbackWrapper.java:99)
       at android.support.v7.app.AppCompatDelegateImpl$AppCompatWindowCallback.onPreparePanel(AppCompatDelegateImpl.java:2576)
       at android.support.v7.app.AppCompatDelegateImpl.preparePanel(AppCompatDelegateImpl.java:1616)
       at android.support.v7.app.AppCompatDelegateImpl.doInvalidatePanelMenu(AppCompatDelegateImpl.java:1869)
       at android.support.v7.app.AppCompatDelegateImpl$2.run(AppCompatDelegateImpl.java:227)
       at android.view.Choreographer$CallbackRecord.run(Choreographer.java:911)
       at android.view.Choreographer.doCallbacks(Choreographer.java:723)
       at android.view.Choreographer.doFrame(Choreographer.java:655)
       at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:897)
       at android.os.Handler.handleCallback(Handler.java:789)
       at android.os.Handler.dispatchMessage(Handler.java:98)
       at android.os.Looper.loop(Looper.java:164)
       at android.app.ActivityThread.main(ActivityThread.java:6938)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:327)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1374)

5cac84cff8b88c29632af4d8-fabric

@designsimply
Copy link
Contributor

7-day impact: ~17 per day
Users affected: 20 per week
First seen in: 12.1.1 on Apr 9
Last seen in: 12.2

(5cac84cff8b88c29632af4d8-fabric-android)

@theck13 any chance this crash could be related to #9421?

@designsimply
Copy link
Contributor

designsimply commented May 2, 2019

@theck13 actually, EditPostActivity references probably means the problem is elsewhere right? 🤦‍♀️

(History for EditPostActivity.java)

@theck13
Copy link
Contributor

theck13 commented May 2, 2019

EditPostActivity is unrelated to #9421. The stack trace suggests the crash is due to a menu item in the editor being used on Line 1188 of the EditPostActivity.java file when the menu item object is null. Any changes to the editor and specifically any changes to the top app bar menu items in the editor could be to blame.

@daniloercoli
Copy link
Contributor

I've investigated this a bit, but was not able to reproduce the problem.

EditPostActivity should always load the correct edit_post menu file in onCreateOptionsMenu, since isModernEditor does always return true, and later there should not be NPE in onPrepareOptionsMenu.

It's not clear to me how in the onPrepareOptionsMenu method call

        MenuItem switchToAztecMenuItem = menu.findItem(R.id.menu_switch_to_aztec);

is null and lead to the crash.

@daniloercoli
Copy link
Contributor

Checked in Fabric, and the issue does affect Android 8 devices only.

@designsimply
Copy link
Contributor

designsimply commented May 9, 2019

7-day impact: ~8 per day
Users affected: 18
Last seen in: 12.2

(5cac84cff8b88c29632af4d8-fabric-android)

Note: Couldn't find a report for this one in Sentry which we switched to as of 12.3. Ignore that last part. Found it! Added a new comment.

@sentry-io
Copy link

sentry-io bot commented May 17, 2019

Sentry issue: WORDPRESS-ANDROID-13

5-day impact: ~7 per day
Users affected: 14 in 5d
Last seen in: 12.3.2

@designsimply
Copy link
Contributor

30-day impact: ~12 per day
Users affected: 104 in the last 30d
First seen in: 12.1.1
Last seen in: 12.4

https://sentry.io/share/issue/3edf4083c6cc4d49a97e153fa45566dc/

@designsimply
Copy link
Contributor

designsimply commented Jul 11, 2019

Removed [Pri] High because it's no longer a new crash. Crashes are prioritized by age and frequency with the newest crashes as the highest priority followed by the crashes happening the most often.

@designsimply
Copy link
Contributor

90-day impact: ~4 per day
Users affected in the last 90 days: 58
First seen in: 12.1.1
Last seen in: 12.6
Limited to: Android 8.0.0

https://sentry.io/share/issue/3edf4083c6cc4d49a97e153fa45566dc/

@maxme maxme assigned maxme and unassigned maxme Sep 11, 2019
@develric
Copy link
Contributor

I investigated this a bit and wasn't able to reproduce also.

From Sentry it is still confirmed that the commonality is Android 8.0.0 and it does not seem a logic issue primarily.

One thing to notice is that (AFAIU) it seems to happen always while we are in Aztec.

It's interesting to notice also that the switchToAztecMenuItem and the switchToGutenbergMenuItem are the only two menu items in the onPrepareOptionsMenu we do not check for null. The NPE happens on switchToAztecMenuItem that is the first not null checked item

Cannot understand if there is a reason behind not checking for null but possibly we can be a bit more on the defensive code side and check also them? We can add a log to trace this event occurrence anyway.

One odd thing I can think of is if caused by strange 8.0.0 behaviours we end up having both switch to Aztec and to Gutemberg menu items visible at the same time (if this odd null menu item happens at first init of the screen for example). Maybe we can add some checks in onOptionsItemSelected to take into account things like this:

// if content has blocks or empty, offer the switch to Gutenberg. The block editor doesn't have good
//  "Classic Block" support yet so, don't offer a switch to it if content doesn't have blocks. If the post
//  is empty but the user hasn't enabled "Use Gutenberg for new posts" in Site Setting,
//  don't offer the switch.

If the above makes sense I can open a PR. @daniloercoli, @planarvoid wdyt?

@planarvoid
Copy link
Contributor

planarvoid commented Sep 25, 2019

we can be a bit more on the defensive code side and check also them?

@develric I think this totally makes sense, let's add the non-null checks there
The second suggestion also sounds good to me. Feel free to open a PR :-)

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

Successfully merging a pull request may close this issue.

7 participants