-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix: CMM-754 bug when loading big images in high res #22235
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: trunk
Are you sure you want to change the base?
Fix: CMM-754 bug when loading big images in high res #22235
Conversation
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.
Pull Request Overview
Fixes a bug where large, complex images were timing out when loading in high resolution by increasing the Glide request timeout from the default to 15 seconds. The change addresses server-side processing delays of 5-10 seconds for complex images.
- Increased Glide request timeout to 15 seconds to handle complex image processing
- Applied timeout configuration using Volley's DefaultRetryPolicy
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
WordPress/src/main/java/org/wordpress/android/networking/GlideRequestFactory.kt
Show resolved
Hide resolved
Generated by 🚫 Danger |
|
App Name | ![]() |
|
Flavor | Jalapeno | |
Build Type | Debug | |
Version | pr22235-189cfcd | |
Commit | 189cfcd | |
Direct Download | jetpack-prototype-build-pr22235-189cfcd.apk |
|
App Name | ![]() |
|
Flavor | Jalapeno | |
Build Type | Debug | |
Version | pr22235-189cfcd | |
Commit | 189cfcd | |
Direct Download | wordpress-prototype-build-pr22235-189cfcd.apk |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #22235 +/- ##
==========================================
- Coverage 39.89% 39.88% -0.01%
==========================================
Files 2164 2164
Lines 102440 102450 +10
Branches 14756 14756
==========================================
Hits 40866 40866
- Misses 58115 58125 +10
Partials 3459 3459 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
): Request<ByteArray>? { | ||
val httpsUrl: String = convertWPcomUrlToHttps(url) | ||
return VolleyStreamFetcher.GlideRequest(httpsUrl, callback, priority, addAuthHeaders(url, headers)) | ||
return VolleyStreamFetcher.GlideRequest( |
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.
I put a breakpoint here and viewed the post, but this was never called for me (and the high-res image never loaded).
I tried deleting the app and reinstalling then trying again, but the issue persists (so I don't think it's something to do with caching).
What would you suggest?
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.
That's odd. They loaded for me after 4 and 6 secs 🤔
ReaderPhotoView image loaded successfully after 4349ms https://wpmobilep2.wordpress.com/wp-content/uploads/2025/09/frame-3.png?w=2400
ReaderPhotoView image loaded successfully after 6202ms https://wpmobilep2.wordpress.com/wp-content/uploads/2025/09/frame-4.png?w=2400
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.
I've added some debug logs. So, could you try it again and send me the trace back? Tag: "ReaderPhotoView"
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.
These aren't getting called either, so I did some searching. reader photo fragment > newInstance
is getting called, so I added similar logs to the top of all the lifecycle methods in that fragment, and it seems none of them are being called?
This is a rather old test device – could that be related?
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.
Odd... What Android version are you running?
Maybe there's an exception to run a different flow for old versions
|
Description
This PR is replacing this other one , and taking a more simplistic approach.
Instead of adding a retry mechanism with a fallback URL, it's just increasing the timeout when loading images with Glide.
Tests showed that complex images were taking from 5 to 10 secs to be processed at the server side, so the timeout has been set to 15 secs to be on the safe side.
Testing instructions