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: Images on iOS are cropped weird #1840

Merged
merged 5 commits into from
Jan 28, 2025

Conversation

Soxasora
Copy link
Member

@Soxasora Soxasora commented Jan 22, 2025

Description

Fixes #1793

My hypothesis is that iOS gives up on decoding and thus loading the image if during asynchronous decoding it runs out of resources, indeed on reload the problem might be resolved (essentially retrying decoding).

If we force synchronous decoding, iOS gives this job more priority by using the main thread, potentially fixing weird artifacts and image misplacement.

Screenshots

image

Additional Context

In a no-cache dev scenario this problem really doesn't exist because after the page is fully loaded, images start loading one by one.

Another possible fix, if this fails, could be to update CSS after all the images are loaded, much like we already do for videos as this fixes it in prod.

There's also another fix that implies reducing the thumbnail resolution to avoid running out of iOS resources

Previously drafted to explore new solutions

Checklist

Are your changes backwards compatible? Please answer below:
Yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
4, issue doesn't present itself in dev

For frontend changes: Tested on mobile, light and dark mode? Please answer below:
n/a

Did you introduce any new environment variables? If so, call them out explicitly here:
No

@Soxasora Soxasora added the bug label Jan 22, 2025
@Soxasora Soxasora marked this pull request as draft January 22, 2025 14:57
@Soxasora Soxasora marked this pull request as ready for review January 22, 2025 14:57
@huumn
Copy link
Member

huumn commented Jan 22, 2025

Did you explore decode() at all? That came up when I was researching the decode attribute. According to MDN, which is occasionally wrong, it appears that safari decodes sync by default (most of the time).

@Soxasora
Copy link
Member Author

Soxasora commented Jan 22, 2025

Ah, yes I drafted the PR earlier to explore better solutions but I didn't deep-dive into decode(), thank you.

it appears that safari decodes sync by default (most of the time)

I was trying with decoding attribute mainly for that, like MDN stated Safari can choose a decoding method opaquely.
But we don't live by hypotheses, I'll redraft and get to it, thanks k00b!

edit: this is the same issue of videos not being loaded, seems that Safari needs redrawing on imgproxied images

@Soxasora Soxasora marked this pull request as draft January 22, 2025 19:51
@Soxasora
Copy link
Member Author

Soxasora commented Jan 28, 2025

Loading the image after successful decoding should potentially fix this issue. We're still in a blind scenario but it's still good practice to do so anyway. Before claiming it as ready for review I'll try reproducing with caching

edit: same results outside nextjs dev, images load correctly so I think we can safely try it

@Soxasora Soxasora marked this pull request as ready for review January 28, 2025 19:42
@huumn
Copy link
Member

huumn commented Jan 28, 2025

Yep, I've confirmed there are no regressions. Let's give a shot.

@huumn huumn merged commit bd84b8b into stackernews:master Jan 28, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Images on iOS are cropped weird
2 participants