Skip to content

Avoid NPE when webeditor is null#12419

Merged
mchowning merged 6 commits intogutenberg/release_1.32_integrationfrom
avoid_npe_when_webeditor_null
Jul 10, 2020
Merged

Avoid NPE when webeditor is null#12419
mchowning merged 6 commits intogutenberg/release_1.32_integrationfrom
avoid_npe_when_webeditor_null

Conversation

@mchowning
Copy link
Copy Markdown
Contributor

@mchowning mchowning commented Jul 10, 2020

When testing unsupported block editing for the gutengerg 1.32 release, I observed that on my pressable site the webEditor was null and this was crashing the app. This addresses that by reversing the .equals() check to avoid the NPE.

UPDATE: This PR was originally fixing a NPE, but it grew to address a newly discovered bug on self-hosted sites, and then eventually to just disable the feature for the time being to allow for more testing.

To test:
Load the gutenberg editor on a site where the webeditor value is null (a Pressable non-jetpack connected site is what I used), and confirm that the app does not crash.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • 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
Copy Markdown

peril-wordpress-mobile bot commented Jul 10, 2020

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@mchowning mchowning added this to the 15.3 milestone Jul 10, 2020
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jul 10, 2020

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

@mchowning mchowning requested a review from marecar3 July 10, 2020 15:20
@mchowning
Copy link
Copy Markdown
Contributor Author

Tested and I am seeing that

  • I can edit unsupported posts with wpcom sites, including atomic.
  • I cannot edit unsupported blocks on self-hosted sites regardless of whether they are connected to jetpack.

I believe this is the expected behavior at this time, so I think this PR is good to merge.

@mchowning
Copy link
Copy Markdown
Contributor Author

mchowning commented Jul 10, 2020

We realized that it is possible for wp.com sites to have the block editor disabled as well, and we have not thoroughly tested that scenario, so a214faf disables the editing unsupported blocks feature since we need to merge the release. We can always update the beta app before it's released if we decide to turn it back on later after some more testing.

UPDATE: Decision was made to go ahead with the feature.

@mchowning mchowning force-pushed the avoid_npe_when_webeditor_null branch from 67e0c0f to 33a07cb Compare July 10, 2020 20:11
@guarani
Copy link
Copy Markdown
Contributor

guarani commented Jul 10, 2020

@mchowning mchowning merged commit a5dc3c1 into gutenberg/release_1.32_integration Jul 10, 2020
@mchowning mchowning deleted the avoid_npe_when_webeditor_null branch July 10, 2020 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants