From 36889c7da35c02723d2f7df61f1ca65e9a283c5e Mon Sep 17 00:00:00 2001 From: develric Date: Fri, 27 Sep 2019 03:16:34 +0200 Subject: [PATCH 1/2] Adding null checks in EditPostActivity onPrepareOptionsMenu --- .../android/ui/posts/EditPostActivity.java | 73 +++++++++++++------ 1 file changed, 49 insertions(+), 24 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java index 021a9bbaa9e2..125d90d5f7d8 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java @@ -1074,6 +1074,40 @@ private void hidePhotoPicker() { } } + private boolean shouldSwitchToGutenbergBeVisible( + PostModel post, + EditorFragmentAbstract editorFragment, + SiteModel site + ) { + // Some guard conditions + if (post == null) { + AppLog.w(T.EDITOR, "shouldSwitchToGutenbergBeVisible got a null post parameter."); + return false; + } + + if (editorFragment == null) { + AppLog.w(T.EDITOR, "shouldSwitchToGutenbergBeVisible got a null editorFragment parameter."); + return false; + } + + // Check whether the content has blocks. + boolean hasBlocks = false; + boolean isEmpty = false; + try { + final String content = (String) editorFragment.getContent(post.getContent()); + hasBlocks = PostUtils.contentContainsGutenbergBlocks(content); + isEmpty = TextUtils.isEmpty(content); + } catch (EditorFragmentNotAddedException e) { + // legacy exception; just ignore. + } + + // 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. + return hasBlocks || (SiteUtils.isBlockEditorDefaultForNewPost(site) && isEmpty); + } + /* * called by PhotoPickerFragment when media is selected - may be a single item or a list of items */ @@ -1224,31 +1258,22 @@ public boolean onPrepareOptionsMenu(Menu menu) { MenuItem switchToAztecMenuItem = menu.findItem(R.id.menu_switch_to_aztec); MenuItem switchToGutenbergMenuItem = menu.findItem(R.id.menu_switch_to_gutenberg); - if (mShowGutenbergEditor) { - // we're showing Gutenberg so, just offer the Aztec switch - switchToAztecMenuItem.setVisible(true); - switchToGutenbergMenuItem.setVisible(false); - } else { - // we're showing Aztec so, hide the "Switch to Aztec" menu - switchToAztecMenuItem.setVisible(false); + // The following null checks should basically be redundant but were added to manage + // an odd behaviour recorded with Android 8.0.0 + // (see https://github.com/wordpress-mobile/WordPress-Android/issues/9748 for more information) + if (switchToAztecMenuItem != null && switchToGutenbergMenuItem != null) { + if (mShowGutenbergEditor) { + // we're showing Gutenberg so, just offer the Aztec switch + switchToAztecMenuItem.setVisible(true); + switchToGutenbergMenuItem.setVisible(false); + } else { + // we're showing Aztec so, hide the "Switch to Aztec" menu + switchToAztecMenuItem.setVisible(false); - // Check whether the content has blocks. - boolean hasBlocks = false; - boolean isEmpty = false; - try { - final String content = (String) mEditorFragment.getContent(mPost.getContent()); - hasBlocks = PostUtils.contentContainsGutenbergBlocks(content); - isEmpty = TextUtils.isEmpty(content); - } catch (EditorFragmentNotAddedException e) { - // legacy exception; just ignore. - } - - // 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. - switchToGutenbergMenuItem.setVisible( - hasBlocks || (SiteUtils.isBlockEditorDefaultForNewPost(mSite) && isEmpty)); + switchToGutenbergMenuItem.setVisible( + shouldSwitchToGutenbergBeVisible(mPost, mEditorFragment, mSite) + ); + } } return super.onPrepareOptionsMenu(menu); From 8d63ca50097dccb00a85d19cf618dfb9f9296ecf Mon Sep 17 00:00:00 2001 From: develric Date: Fri, 27 Sep 2019 03:35:03 +0200 Subject: [PATCH 2/2] Adding state checks in onOptionsItemSelected and Sentry log --- .../android/ui/posts/EditPostActivity.java | 40 ++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java index 125d90d5f7d8..063c3dfc2dd0 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java @@ -1392,22 +1392,42 @@ public void onClick(View view) { }); } } else if (itemId == R.id.menu_switch_to_aztec) { - // let's finish this editing instance and start again, but not letting Gutenberg be used - mRestartEditorOption = RestartEditorOptions.RESTART_SUPPRESS_GUTENBERG; - mPostEditorAnalyticsSession.switchEditor(Editor.CLASSIC); - mPostEditorAnalyticsSession.setOutcome(Outcome.SAVE); - savePostAndOptionallyFinish(true); + // The following boolean check should be always redundant but was added to manage + // an odd behaviour recorded with Android 8.0.0 + // (see https://github.com/wordpress-mobile/WordPress-Android/issues/9748 for more information) + if (mShowGutenbergEditor) { + // let's finish this editing instance and start again, but not letting Gutenberg be used + mRestartEditorOption = RestartEditorOptions.RESTART_SUPPRESS_GUTENBERG; + mPostEditorAnalyticsSession.switchEditor(Editor.CLASSIC); + mPostEditorAnalyticsSession.setOutcome(Outcome.SAVE); + savePostAndOptionallyFinish(true); + } else { + logWrongMenuState("Wrong state in menu_switch_to_aztec: menu should not be visible."); + } } else if (itemId == R.id.menu_switch_to_gutenberg) { - // let's finish this editing instance and start again, but let GB be used - mRestartEditorOption = RestartEditorOptions.RESTART_DONT_SUPPRESS_GUTENBERG; - mPostEditorAnalyticsSession.switchEditor(Editor.GUTENBERG); - mPostEditorAnalyticsSession.setOutcome(Outcome.SAVE); - savePostAndOptionallyFinish(true); + // The following boolean check should be always redundant but was added to manage + // an odd behaviour recorded with Android 8.0.0 + // (see https://github.com/wordpress-mobile/WordPress-Android/issues/9748 for more information) + if (shouldSwitchToGutenbergBeVisible(mPost, mEditorFragment, mSite)) { + // let's finish this editing instance and start again, but let GB be used + mRestartEditorOption = RestartEditorOptions.RESTART_DONT_SUPPRESS_GUTENBERG; + mPostEditorAnalyticsSession.switchEditor(Editor.GUTENBERG); + mPostEditorAnalyticsSession.setOutcome(Outcome.SAVE); + savePostAndOptionallyFinish(true); + } else { + logWrongMenuState("Wrong state in menu_switch_to_gutenberg: menu should not be visible."); + } } } return false; } + private void logWrongMenuState(String logMsg) { + AppLog.w(T.EDITOR, logMsg); + // Lets record this event in Sentry + CrashLoggingUtils.logException(new IllegalStateException(logMsg), T.EDITOR); + } + private void showEmptyPostErrorForSecondaryAction() { String message = getString(mIsPage ? R.string.error_publish_empty_page : R.string.error_publish_empty_post); if (getSecondaryAction() == SecondaryAction.SAVE_AS_DRAFT || getSecondaryAction() == SecondaryAction.SAVE) {