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

The avatars in users suggestions list not loading #21155

Closed
kean opened this issue Jul 20, 2023 · 6 comments · Fixed by #21169
Closed

The avatars in users suggestions list not loading #21155

kean opened this issue Jul 20, 2023 · 6 comments · Fixed by #21169

Comments

@kean
Copy link
Contributor

kean commented Jul 20, 2023

Expected behavior

  • Avatars are loaded

Actual behavior

  • Avatars are not loaded

Steps to reproduce the behavior

  • Open one of the A8C posts
  • Tap on a comment text field
  • Start typing a username
Tested on [device], iOS [version], Jetpack iOS / WordPress iOS [version]

latest

RCA

  • Take the following URL: https://0.gravatar.com/avatar/6e53a25107fdb57cb66d87fc0c88ff5ba3aa16909ad94e39a825fba47157d08d?s=96&d=identicon&r=G
  • [WPAvatarSource parseURL:forAvatarHash:] is called to determine the avatar size
  • if ([hash length] == 32) { condition resolves to false because hash length is 64
  • SuggestionsTableView/fetchIcon (line 192) doesn't start the download because the resolved image type is .unknown
Screenshot 2023-07-20 at 5 08 06 PM
@kean
Copy link
Contributor Author

kean commented Jul 20, 2023

Hey, @salimbraksa and @guarani, are you familiar with this code? I've been trying to identify where WPAvatarSource is used, but it looks like it's never used for image loading because of this issue.

@kean kean added the Reader label Jul 20, 2023
@kean
Copy link
Contributor Author

kean commented Jul 20, 2023

I'm not sure why it's the only place that uses WPAvatarSource. The other parts of the Reader don't have this issue. The suggestions list should probably use the same image loader with the same cache.

@guarani
Copy link
Contributor

guarani commented Jul 20, 2023

Hey @kean, I'm not familiar with this code but I found this: pb6Nl-gyD-p2.

@kean
Copy link
Contributor Author

kean commented Jul 21, 2023

Thanks, @guarani. Do you know if there is a plan to add support for these new hashes in the app?
This type of change should not break the app...

@guarani
Copy link
Contributor

guarani commented Jul 21, 2023

Now that you discovered the avatars are broken, I think fixing it should be a priority and aligns with what I see in the above link. Does it look like a quick fix?

@kean
Copy link
Contributor Author

kean commented Jul 21, 2023

There is a quick fix (add [hash length] == 64 check), which I already verified.

Step two should be to investigate why only this screen uses WPAvatarSource, replace it with a solution that's used in other parts of the app, and remove WPAvatarSource (with another layer of separate caches).

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

Successfully merging a pull request may close this issue.

2 participants