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

Prevent NPE when session tracker has not been initialized #9497

Merged
merged 1 commit into from
Apr 1, 2019

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Mar 31, 2019

Fixes #9494

Added null check as tracker may not have been initialized if site or Post were null in the first place and Editor was exited preemptively

To test:

  1. artificially make Post or Site null in onCreate(), or add a call to showErrorAndFinish() before the call to createPostEditorAnalyticsSessionTracker()
  2. run and open the Editor
  3. observe it exits but doesn't crash

Update release notes:

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

…Post were null in the first place and Editor was exited preemptively
@mzorz mzorz added this to the 12.2 milestone Mar 31, 2019
@mzorz mzorz requested a review from jtreanor March 31, 2019 16:55
@mzorz mzorz changed the base branch from develop to release/12.1 March 31, 2019 16:56
@mzorz mzorz modified the milestones: 12.2, 12.1 ❄️ Mar 31, 2019
@mzorz mzorz changed the base branch from release/12.1 to develop March 31, 2019 16:56
@mzorz mzorz modified the milestones: 12.1 ❄️, 12.2 Mar 31, 2019
@mzorz
Copy link
Contributor Author

mzorz commented Mar 31, 2019

hi @jtreanor - let me know if the base branch needs be changed, we can create another branch and cherry pick it branching off from release/12.1 if needed 👍

@jtreanor jtreanor changed the base branch from develop to release/12.1 April 1, 2019 08:02
@jtreanor jtreanor changed the base branch from release/12.1 to develop April 1, 2019 08:02
@jtreanor
Copy link
Contributor

jtreanor commented Apr 1, 2019

Thanks for the quick fix @mzorz!

My preference would be to target release/12.1 with this. I tried changing the base but this branch is ahead. Cherry picking once merged is also fine.

Copy link
Contributor

@jtreanor jtreanor left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks Mario!

I'll let you decide if you want to change base to release/12.1 or merge and cherry pick.

@mzorz mzorz merged commit 7b4521c into develop Apr 1, 2019
@mzorz mzorz deleted the issue/9494-fix-npe-editor-session-tracker-ondestroy branch April 1, 2019 11:26
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.

2 participants