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

Fix crash when attempting to render Gutenberg comment #1383

Merged
merged 7 commits into from
Feb 1, 2024

Conversation

itsmeichigo
Copy link
Contributor

Fixes woocommerce/woocommerce-ios#11770
Reported in peaMlT-p5-p2

Description

There's a recent crash found in Woo iOS happening in CommentAttachmentRenderer when attempting to render an image with an illegal size (-1, -1).

This PR fixes this crash with two steps:

  • Added a guard check to ensure the image size is acceptable before sending it to UIGraphicsBeginImageContextWithOptions
  • Move the guard check for Gutenberge comment on the top before proceeding to render an image.

Testing steps

When trying to reproduce the crash, I found that this happens when a product description in Woo contains some Gutenberg comments. I added similar comments to the Example/Example/SampleContent/content.html file, so you can test with the example app by tapping Standard demo and confirm that the app doesn't crash.


  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

@itsmeichigo itsmeichigo marked this pull request as ready for review January 31, 2024 10:57
Bump podspec version for WordPress-Editor-iOS

Update CHANGELOG
@itsmeichigo itsmeichigo force-pushed the fix/crash-comment-attachment-renderer branch from 5d89e71 to b0535fb Compare January 31, 2024 11:02
@itsmeichigo itsmeichigo changed the title Fix crash due when attempting to render Gutenberg comment Fix crash when attempting to render Gutenberg comment Jan 31, 2024
@hafizrahman hafizrahman self-assigned this Feb 1, 2024
@itsmeichigo itsmeichigo force-pushed the fix/crash-comment-attachment-renderer branch from 4114d33 to 4049aa5 Compare February 1, 2024 05:58
Copy link

@hafizrahman hafizrahman left a comment

Choose a reason for hiding this comment

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

Nice fix! Great that you add the size check too.

I also was able to replicate the crash by testing the content.html change here with trunk build, perfectly matches the Sentry report trace in woocommerce/woocommerce-ios#11770:

Screenshot 2024-02-01 at 14 50 25

LGTM 👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants