-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: prevent SVG crash on iOS and enable rendering on Android #6949
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
base: develop
Are you sure you want to change the base?
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughWhen message image status is Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/containers/message/Components/Attachments/Image/Image.tsx (1)
26-38: Add a.catch()handler to prevent unhandled promise rejections.The
maxHeightandmaxWidthoptions are correctly supported in[email protected]and are the recommended approach for preventing memory exhaustion from large SVGs on iOS. However, the promise chain lacks a.catch()handler, which could result in unhandled promise rejections if image loading fails:Suggested improvement
useEffect(() => { if (status === 'downloaded') { Image.loadAsync(uri, { onError: e => { log(e); }, maxHeight: 1000, maxWidth: 1000 - }).then(image => { + }) + .then(image => { setImageDimensions({ width: image.width, height: image.height }); - }); + }) + .catch(e => { + log(e); + }); } }, [uri, status]);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/containers/message/Components/Attachments/Image/Image.tsx`:
- Around line 28-36: The Image.loadAsync call is using hardcoded maxHeight: 100
and maxWidth: 100 which caps iOS-loaded images to tiny thumbnails; update the
Image.loadAsync options in the Image.loadAsync(...) call so it does not hardcode
100px—either remove maxHeight/maxWidth entirely or replace them with a
computed/layout-aware value (e.g. reuse the maxSize pattern from Urls.tsx) and
only apply restrictive max dimensions for SVG mime-types if needed; ensure
setImageDimensions({ width: image.width, height: image.height }) receives the
full/respected dimensions and test on both iOS and Android.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/containers/message/Components/Attachments/Image/Image.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:06.688Z
Learning: The RocketChat/Rocket.Chat.ReactNative repository uses a fork of react-native-image-crop-picker (RocketChat/react-native-image-crop-picker) with custom Android edge-to-edge fixes, not the upstream ivpusic/react-native-image-crop-picker package. Dependencies should reference commit pins from the RocketChat fork.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
OtavioStasiak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Run e2e tests and you can merge it :)
Proposed changes
This pull request fixes issues related to SVG image handling in channel messages.
iOS app crashed when a room contained an SVG image, and Android failed to render SVG images altogether. This update ensures consistent and stable behavior across both platforms by preventing the crash on iOS and enabling proper SVG rendering on Android.
Issue(s)
Closes: #6947
https://rocketchat.atlassian.net/browse/CORE-1764
https://rocketchat.atlassian.net/browse/CORE-1812
How to test or reproduce
Screenshots
SVG image is not crashing iOS App and rendering without any issue

Types of changes
Checklist
Further comments
Took reference from expo/expo#38323
Summary by CodeRabbit