Visible tile invalidation flicker on application restoration

VERIFIED FIXED

Status

()

Firefox for iOS
General
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: Ninu, Assigned: sleroux)

Tracking

unspecified
Other
iOS

Firefox Tracking Flags

(fxios-v5.0 verified, fxios-v6.0 verified, fxios6.0+)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Build version 4.0.0(7)
Device: iPhone 6Plus 
Firmware: 9.1 

Steps to reproduce:
1.Have multiple tiles in Top sites tab 
2.Press home button and resume the app 
3.Repeat step 2 multiple times 

Result: The tile refresh has an improper graphic feedback, and it refreshes at every resume with this graphic glitch, even though no changes were made to the tile arrangement.
For further details please check: https://www.youtube.com/watch?v=KOVHYXcP9tQ&feature=youtu.be

Please note that this issue could be reproduced only on the iPhone 6Plus with fw 9.1. could not be reproduced on a iPhone 6(fw 9.2.1), iPad Mini 4 (fw 9.1) and iPod Touch 6th gen (fw 9.1)
Brian is this fixed with https://github.com/mozilla/firefox-ios/pull/1773/commits/dcba7f1ef6d949e542b2c5ea0e8abb9546055893? Did that make it to 4.0 (7)? What sort of tile invalidation do we do when the application is restored to the foreground here?
tracking-fxios: --- → ?
Flags: needinfo?(bmunar)
Summary: Top site panel will refresh at every app resume with improper graphic display → Visible tile invalidation flicker on application restoration
I think this should be fixed on master because we were updating the UI in two different places. Now I only update it when we get the data from the user's profile and, as far as I can tell, this doesn't get shown when I test it on the simulator. Would love it if we could have another pair of eyes to see if the patch fixes this!
Flags: needinfo?(bmunar)
does this still happen?
Flags: needinfo?(mihai.ninu)
Tested on latest master 4593dfdc.

The issue is still reproducible following the steps in the description.
Flags: needinfo?(mihai.ninu)
Flags: needinfo?(sarentz)
Flags: needinfo?(bmunar)
Assignee: nobody → bmunar
Flags: needinfo?(sarentz)
Bryan, bouncing this back to you. Do you think this is something you can address for v5.0?
Flags: needinfo?(bmunar)
For record, during last triage Stefan mentioned that he saw the default tiles flickering behind his top-sites
tracking-fxios: ? → 6.0+
The issue was reproduced on latest master 8bb33eb, IPhone SE (9.3.2) and IPhone 6 S (9.2.1) while signed in to FXA. Please see the attached URL for more information: https://youtu.be/3gxr_1dQhuw.
(Assignee)

Comment 8

2 years ago
Created attachment 8764642 [details]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1939#attch-to-bugzilla

Hey Bryan,

I kept on seeing this and figured I would take a peek - I hope you don't mind! Turns out we were always invalidating when re-entering the app because the sync finish notification fires, needlessly invalidating top sites. I've added a method to check if the cache has actually changed from the result of a sync.
Attachment #8764642 - Flags: review?(etoop)
Attachment #8764642 - Flags: review?(bmunar)
Comment on attachment 8764642 [details]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1939#attch-to-bugzilla

Deal with the NotificationDynamicFontChanged case and it looks good. Nice check.
Attachment #8764642 - Flags: review?(etoop) → review+

Updated

2 years ago
Assignee: bmunar → sleroux
Comment on attachment 8764642 [details]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1939#attch-to-bugzilla

lgtm (don't really know much about the SQL stuff tho; haven't done any of that kind of stuff really)
Attachment #8764642 - Flags: review?(bmunar) → review+
(Assignee)

Comment 11

2 years ago
master 8268720d9bab2c72d0792b21c4d9c960d442bc6e
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-fxios-v5.0: --- → affected
status-fxios-v6.0: --- → fixed
Resolution: --- → FIXED
Whiteboard: [needsuplift]
(Assignee)

Comment 12

2 years ago
v5.x c23c68f
status-fxios-v5.0: affected → fixed
Whiteboard: [needsuplift]

Updated

2 years ago
Duplicate of this bug: 1277227
Verifying as fix on bd689d7e33.
Status: RESOLVED → VERIFIED
status-fxios-v5.0: fixed → verified
status-fxios-v6.0: fixed → verified
You need to log in before you can comment on or make changes to this bug.