Page MenuHomePhabricator

[BUG] Images flash in app upon foreground
Closed, ResolvedPublicBUG REPORT

Description

Steps to reproduce

  1. Background the app
  2. Foreground the app

Expected results

Images do not flash

Actual results

Images in the app (Explore, Saved Articles, History) flash, as if all the cells are reloading when they don't need to.

Environments observed

App version: 7.7.1 (4944), but not a new bug
OS versions: 18.3.1
Device model: iPhone 12 mini
Device language: English

Related Objects

Event Timeline

Thank you for tagging this task with good first task for Wikimedia newcomers!

Newcomers often may not be aware of things that may seem obvious to seasoned contributors, so please take a moment to reflect on how this task might look to somebody who has never contributed to Wikimedia projects.

A good first task is a self-contained, non-controversial task with a clear approach. It should be well-described with pointers to help a completely new contributor, for example it should clearly pointed to the codebase URL and provide clear steps to help a contributor get setup for success. We've included some guidelines at https://phabricator.wikimedia.org/tag/good_first_task/ !

Thank you for helping us drive new contributions to our projects <3

Engineering notes:

All the mentioned views use cells that subclass ArticleCollectionViewCell. Investigate the issue with image loading on the cell and collection views.

@Mazevedo This issue persists not only when bringing the app to the foreground but also on initial load/reload. The images on the first screen after setup/reset flicker, which suggests the issue may be related to how cells are configured or reloaded.
Would you be able to help me point to the code responsible for this behaviour? Any guidance on the best approach to fix it would be greatly appreciated!

Hi @Anvitha098, as I mentioned in the comment above, all these parts of the app subclass ArticleCollectionViewCell .

You can start by investigating this class's setup() and reset() methods to check if they are being called correctly.
If the issue is not on this class specifically, you can check the classes subclassing it. They usually have configure(...) methods, where the images are assigned to each cell. Those methods will be called from the view controllers instantiating these cells on the collection view data source delegate methods like collectionView(_ collectionView:, cellForItemAt indexPath:).
That might also be a good spot for investigating - check if the view controller is calling this method when it shouldn't.

Let me know if it leaves you in a better spot to investigate this issue.

Mazevedo added a subscriber: Anvitha098.

I think the root cause here is that when the app is entering the background or foreground, [WMFAppViewController -traitCollectionDidChange:] gets called, and [WMFAppViewController updateAppThemeIfNecessary] gets called, since the traitCollection address has changed, [WMFAppViewController appEnvironmentTraitCollectionIsDifferentThanTraitCollection:] will return true, so all of these applyTheme: gets called every time, even if the values are identical.

image.png (376×1 px, 228 KB)

During a theme change, collection view cell gets reloaded in ColumnarCollectionViewController and the image views get reset. Since NSCache will evict objects when the app enters the background, reapplying themes will eventually cause images flashing.

So to solve this specific issue, I'm thinking to modify [WMFAppViewController appEnvironmentTraitCollectionIsDifferentThanTraitCollection:] to compare its values, not the object itself.

https://github.com/wikimedia/wikipedia-ios/pull/5412

Changes Made Across Different Files:

The changes I made across multiple collection view cell files follow a consistent pattern. In each file, I modified the reset() method to add a conditional check before calling wmf_reset():
Before the changes:

override func reset() {

super.reset()
imageView.wmf_reset()  // Always called
// ... other reset code

}

After the changes:
override func reset() {

super.reset()
// Only reset the image if it's not already loaded to prevent flashing
if imageView.image == nil {
    imageView.wmf_reset()
}
// ... other reset code

}

Files that I modified:

  1. ImageCollectionViewCell.swift
  2. ArticleCollectionViewCell.swift
  3. AnnouncementCollectionViewCell.swift
  4. ArticlePlaceView.swift - handles two image views
  5. ArticleCollectionViewCell+ListDisplay.swift
  6. InsertMediaSearchResultCollectionViewCell.swift
  7. SideScrollingCollectionViewCell.swift
  8. ExploreCardViewController.swift
  9. CollectionViewUpdater.swift

@Harsh_Kushwaha07: Hi, please provide a patch instead; no need for videos of IDEs or explanations about changes in individual files here. Thanks!

@Harsh_Kushwaha07 Great work for finding all these affected cells, but I don't think this is a good approach. Resetting all its content to its original state for cell reuse is the goal here. Skipping image-resetting for reuse, which violates this and could cause issues, such as if the new cell’s image is damaged or its link is unreachable, then it will display the wrong image.

ABorbaWMF subscribed.

Testing on 7.8.2 (5803)

This is still occurring as described in the ticket.

Hi @ABorbaWMF, you may be in an older build. Can you test on 7.8.2 (5826)? Thanks!

@Tsevener - No problem, but we have 7.8.2 (5803) listed here T403740. So we may need to change that if the expectation is to test on 7.8.2 (5826)

Looks good on 7.8.2 (5826)

Tested on iPhone 16 on iOS 26 and iPad Pro 12.9 on iOS 26. Tested multiple screens with multiple images

@ABorbaWMF Thanks for the callout! We'll put in (test latest) in the release task. We may have one or two more builds trickle in this week as we merge so generally a good idea to always test the latest until we submit it for review. For the smoketest task we will always be specific to which build number. Happy to discuss further synchronously if you'd like.