search

wordpress-mobile / AztecEditor-Android

A reusable native Android rich text editor component.
Mozilla Public License 2.0
673 stars 114 forks source link

Address `NullPointerException` crash in `AztecHeadingSpan` #1071

Closed SiobhyB closed 5 months ago

SiobhyB commented 5 months ago

A potential fix for the crash described at https://github.com/wordpress-mobile/WordPress-Android/issues/18657.

Related PRs

Description

Sentry has been reporting the following crash for both the WordPress and Jetpack Android apps:

NullPointerException
org.wordpress.aztec.spans.AztecHeadingSpan in chooseHeight

The stack trace for each individual event specifically references one of the following lines (the specific line varies for each event), all of which are within the chooseHeight function and include a reference to previousFontMetrics:

As described at https://github.com/wordpress-mobile/WordPress-Android/issues/18657#issuecomment-1835995579, it has proven difficult to reproduce this crash. It's suspected that there are specific instances with lower performing devices or devices on poor connections that can lead to previousFontMetrics being set as null elsewhere in the code while chooseHeight is running.

Given the difficulty reproducing, it hasn't been possible to get to the heart of this issue. Instead, with this PR, the previous non-null assertions previously used with previousFontMetrics have been replaced with safe calls. With this approach, the functionality should remain the same, while guarding against NullPointerException crashes.

Test

Verify Aztec demo continues to function as expected

  1. Load the Aztec demo with this PR's changes applied.
  2. Verify that you can add headings with no unexpected side effects.

Verify Gutenberg continues to function as expected

  1. Using the installable build available from https://github.com/wordpress-mobile/WordPress-Android/pull/19724, install the app and navigate to the post editor.
  2. Add some heading blocks and ensure there are no unexpected side effects when performing tasks. Example tasks to test include splitting the block, applying new font sizes and line heights, etc.
SiobhyB commented 5 months ago

@khaykov, 👋 , I wanted to give you a heads up on this PR as I know Day One uses Aztec for the Android app. Can you take a look to let me know if you spot anything that might cause issues for you? We're hoping to merge this fix in time for a beta in the next day or so.

SiobhyB commented 5 months ago

@khaykov, I'm going to go ahead to merge this PR, so we can get it ready for an upcoming beta. Please let me know if there's anything that stands out to you as off, though! I think the changes are fairly safe, but thought it was best to make you aware of them regardless.