1270765 - Visible tile invalidation flicker on application restoration

Status: VERIFIED FIXED

(Firefox for iOS :: General, defect)

Description

[u549602]

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)

Comment 1

[Aaron Train [:aaronmt]]

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?

Comment 2

[Bryan Munar (:bkmunar)]

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!

Comment 3

[Bryan Munar (:bkmunar)]

does this still happen?

Comment 4

[Simon Basca [:SimonB]]

Tested on latest master 4593dfdc.

The issue is still reproducible following the steps in the description.

Comment 5

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Bryan, bouncing this back to you. Do you think this is something you can address for v5.0?

Comment 6

[Aaron Train [:aaronmt]]

For record, during last triage Stefan mentioned that he saw the default tiles flickering behind his top-sites

Comment 7

[Simon Basca [:SimonB]]

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.

Comment 8

[Stephan Leroux [:sleroux]]

Attachment: 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.

Comment 9

[Emily Toop (:fluffyemily)]

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.

Comment 10

[Bryan Munar (:bkmunar)]

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)

Comment 11

[Stephan Leroux [:sleroux]]

master 8268720d9bab2c72d0792b21c4d9c960d442bc6e

Comment 12

[Stephan Leroux [:sleroux]]

v5.x c23c68f

Comment 14

[Simon Basca [:SimonB]]

Verifying as fix on bd689d7e33.