Skip to content

Conversation

adalpari
Copy link
Contributor

@adalpari adalpari commented Sep 15, 2025

Description

When loading a specific post with high-res images, some of them are not loaded properly, and they look blurry.

It turned out the problem was on the image itself, so Volley is throwing a Timeout Exception. In this PR I've added a new mechanism to retry loading the image with a lower resolution. The problem is that it takes some time to detect the timeout and reload. And increasing the timeout will just cause more UX problems I guess...

Something feels off with the image as it's not especially a heavy one. These are the two URLS:
https://[BLURRED].wordpress.com/wp-content/uploads/2025/09/frame-3.png?w=1920
https://[BLURRED].wordpress.com/wp-content/uploads/2025/09/frame-4.png?w=1920
One is loading in less than 100ms, and the other one is throwing a TimeoutException after 7secs...

Testing instructions

  1. Open the post in here: https://a8c.slack.com/archives/C08LE4U7FAM/p1757697088240459
  2. Open the first image
  3. Wait until it's decently loaded

(Video in the Slack thread because of internal info)

@dangermattic
Copy link
Collaborator

dangermattic commented Sep 15, 2025

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@adalpari adalpari requested a review from Copilot September 15, 2025 12:46
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a bug where high-resolution images fail to load properly in the Reader photo viewer due to timeout exceptions. The fix implements a fallback mechanism that retries loading with lower resolution when timeouts occur.

  • Adds a 5-second timeout to image loading requests
  • Implements fallback loading with 50% resolution when high-res images fail
  • Reduces initial high-resolution width calculation to 80% of screen size

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
ImageManager.kt Adds timeout configuration and imports timeout utilities
ReaderPhotoView.java Implements fallback image loading mechanism with lower resolution retry
ReaderPhotoViewerFragment.java Reduces high-resolution image width to prevent oversized requests

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 15, 2025

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr22211-ab1be7f
Commitab1be7f
Direct Downloadwordpress-prototype-build-pr22211-ab1be7f.apk
Note: Google Login is not supported on these builds.

@adalpari adalpari marked this pull request as ready for review September 15, 2025 13:23
@adalpari adalpari requested a review from jkmassel September 15, 2025 13:23
void onTapPhotoView();
}

private String mImageUrl;

Check notice

Code scanning / Android Lint

Nullable/NonNull annotation missing on field Note

Missing null annotation
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 15, 2025

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr22211-ab1be7f
Commitab1be7f
Direct Downloadjetpack-prototype-build-pr22211-ab1be7f.apk
Note: Google Login is not supported on these builds.

@adalpari
Copy link
Contributor Author

@jkmassel I've now added the original image URL, and it seems to be working fine. So, it confirms that there's some kind of issue loading the redimensioned version. Who should I speak with to raise a flag here?

I've also added tracking for it.

@adalpari
Copy link
Contributor Author

Update note: I tried to set the already loaded image as the placeholder for the fullscreen view, so the transition would be smoother while loading the highRes. But unfortunately, I realised that the reader is a webview, so there's no way to improve the UX from the client side by following that path. So, definitely server improvement will benefit the experience.

Copy link

@adalpari
Copy link
Contributor Author

Closing in favor of: #22235

@adalpari adalpari closed this Sep 25, 2025
auto-merge was automatically disabled September 25, 2025 17:00

Pull request was closed

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

Successfully merging this pull request may close these issues.

3 participants