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

[iOS] Add png extension only if file exist when load local image #46971

Closed

Conversation

zhongwuzw
Copy link
Contributor

Summary:

FIxes #46870.

To avoid many breaking changes, I modified the logic to add the PNG extension only if the file exists.

Changelog:

[IOS] [FIXED] - Add png extension only if file exist when load local image

Test Plan:

Demo in #46870 .

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 11, 2024
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 11, 2024
@cortinico cortinico requested a review from cipolleschi October 15, 2024 12:43
@fbp93
Copy link

fbp93 commented Oct 22, 2024

@zhongwuzw news? 🙏

@zhongwuzw
Copy link
Contributor Author

@zhongwuzw news? 🙏

Hi, let me try to find someone to review.

@zhongwuzw
Copy link
Contributor Author

@fbp93 Hi, sorry for the delayed response. I contacted the RN team, and this week they’re busy with the 0.76 release and also have a team summit, so things will be slower than usual.

@fbp93
Copy link

fbp93 commented Oct 23, 2024

@fbp93 Hi, sorry for the delayed response. I contacted the RN team, and this week they’re busy with the 0.76 release and also have a team summit, so things will be slower than usual.

ok thanks 👍 No worries, in the meanwhile I'm creating a patch with the fix and I'm using that one

@cipolleschi
Copy link
Contributor

I'm a bit confused. This code is 5 years old, how come it stopped working recently?
I think that there should be another reason behind the failure.

That said, the fix looks ok. It seems reasonable to add the extension only if the file exists.

Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

See my comment above.

@zhongwuzw
Copy link
Contributor Author

@cipolleschi Hi, #37232 leads to the issue, we changes the sandbox file loading mechanism , and uses URLRequest to load the file image instead. So back to this PR, seems it's also reasonable to check whether file exist before add png extension. :)

Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Thanks for digging into the issue and for working on the fix.

@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Nov 4, 2024
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in 44f2a08.

cipolleschi added a commit to cipolleschi/react-native that referenced this pull request Jan 23, 2025
Summary:
We have a report from OSS where Images are not displayed properly in case they are saved on disk with no extension.

We previously had a fix attempt iwith [this pr](facebook#46971), but this was breaking some internal apps.

This second attempt should work for both cases.

## Changelog:
[iOS][Fixed] - Load images even when the extension is implicit

Differential Revision: D68555813
facebook-github-bot pushed a commit that referenced this pull request Jan 23, 2025
Summary:
Pull Request resolved: #48888

We have a report from OSS where Images are not displayed properly in case they are saved on disk with no extension.

We previously had a fix attempt iwith [this pr](#46971), but this was breaking some internal apps.

This second attempt should work for both cases.

## Changelog:
[iOS][Fixed] - Load images even when the extension is implicit

Reviewed By: cortinico

Differential Revision: D68555813

fbshipit-source-id: bc25970aafe3e6e5284163b663d36e00b3df3d82
fabriziocucci pushed a commit that referenced this pull request Jan 27, 2025
Summary:
Pull Request resolved: #48888

We have a report from OSS where Images are not displayed properly in case they are saved on disk with no extension.

We previously had a fix attempt iwith [this pr](#46971), but this was breaking some internal apps.

This second attempt should work for both cases.

## Changelog:
[iOS][Fixed] - Load images even when the extension is implicit

Reviewed By: cortinico

Differential Revision: D68555813

fbshipit-source-id: bc25970aafe3e6e5284163b663d36e00b3df3d82
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.75.4: Local image not displayed by <Image /> when extension is not explicit
4 participants