Skip to content

Prevents crashing when the titleLayout is not initialised - #19665

Merged
antonis merged 1 commit into
release/23.7from
fix/19660-postlist-null-crash
Nov 24, 2023
Merged

Prevents crashing when the titleLayout is not initialised#19665
antonis merged 1 commit into
release/23.7from
fix/19660-postlist-null-crash

Conversation

@antonis

@antonis antonis commented Nov 24, 2023

Copy link
Copy Markdown
Contributor

Fixes #19660

Description

This PR prevents the app from crashing when the titleLayout has not yet been initialised in the onPreDraw of the post list row.

Note: I wasn't able to recreate the conditions of the crash thus I consider that this might be an extreme scenario and handled the null case causing the crash. The crash seems to occur after closing the editor on the posts list. I've tried different scenarios with long and short titles, and with long post list so that the d/WordPress-API: : ListStore: Loading next page call is triggered as indicated from the Sentry breadcrumb.

java.lang.NullPointerException: Attempt to invoke virtual method 'int android.text.Layout.getLineCount()' on a null object reference
    at org.wordpress.android.ui.posts.PostListItemViewHolder$setBasicValues$1.onPreDraw(PostListItemViewHolder.kt:136)

To test the behaviour of the app when the titleLayout is null I hardcoded the value (L:134) and the app behaved normally with the only side effect being to render only one line for the excerpt.


To Test:

  1. Open the posts list
  2. Create a new post or open an existing one
  3. Close the editor
  4. Verify that no crash occurs

Regression Notes

  1. Potential unintended areas of impact

    • Posts list
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    • Manual testing
  3. What automated tests I added (or what prevented me from doing so)

    • The fix is on the UI code

PR Submission Checklist:

  • I have completed the Regression Notes.
  • 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.

UI Changes Testing Checklist:

  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • Talkback.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • Large and small screen sizes. (Tablet and smaller phones)
  • Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

@antonis antonis added this to the 23.7 ❄️ milestone Nov 24, 2023
@antonis
antonis marked this pull request as ready for review November 24, 2023 08:07
@wpmobilebot

Copy link
Copy Markdown
Contributor
WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr19665-1807a0d
Commit1807a0d
Direct Downloadwordpress-prototype-build-pr19665-1807a0d.apk
Note: Google Login is not supported on these builds.

@wpmobilebot

Copy link
Copy Markdown
Contributor
Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr19665-1807a0d
Commit1807a0d
Direct Downloadjetpack-prototype-build-pr19665-1807a0d.apk
Note: Google Login is not supported on these builds.

@peril-wordpress-mobile

Copy link
Copy Markdown
Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@thomashorta thomashorta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix @antonis, the code looks good to me. I was also not able to reproduce it but the code seems to avoid the crash when hardcoding the null value, with the side-effect you mentioned in the PR description.

:shipit:

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.

NullPointerException: Attempt to invoke virtual method 'int android.text.Layout.getLineCount()' on a null object reference

3 participants