Closed Bug 919234 Opened 7 years ago Closed 7 years ago

Regression: Unpinning a site flashes thumbnail into neighbour thumbnail spot

Categories

(Firefox for Android :: General, defect)

27 Branch
ARM
Android
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 27
Tracking Status
firefox26 --- verified
firefox27 --- verified
fennec 26+ ---

People

(Reporter: aaronmt, Assigned: mcomella)

References

Details

(Keywords: regression, reproducible)

Attachments

(1 obsolete file)

Currently when one unpins a site, the thumbnail will flash into it's neighbour spot. See video: http://www.youtube.com/watch?v=NqQqztgVjsM

--
Nightly (09/21)
LG Nexus 4 (Android 4.3)
Regression from new-new-about-home

hg.mozilla.org/mozilla-central/pushloghtml?fromdate=2013-09-20&todate=2013-09-21
Blocks: new-new-about-home
No longer blocks: new-about-home
Summary: Unpinning a site flashes thumbnail into neighbour thumbnail spot → Regression: Unpinning a site flashes thumbnail into neighbour thumbnail spot
tracking-fennec: ? → 26+
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
> Our most likely culprit:

I lied, as Aaron mentioned comment 1, this is the whole new-new-about-home change so something was probably lost in translation (e.g. [1]).

[1]: https://hg.mozilla.org/mozilla-central/rev/a7921e0be999
Yeah not sure of the exact changeset, but it was introduced during the work week.
(In reply to Aaron Train [:aaronmt] from comment #4)
> Yeah not sure of the exact changeset, but it was introduced during the work
> week.

This probably related to the fact that we refresh the list of topsites when you unpin a site. But the refresh is supposed to happen in one go. The glitch seems to be caused by some sort of intermediary state before the grid finally settles on the new list of sites.
I wonder if we could use a hack here :) Pull the items that would be in the grid out of the cursor into an Array and have the Adapter look in that array for these items instead of in the cursor.
Comment on attachment 818168 [details] [diff] [review]
Part 1: Do not restart Loader when thumbnails load.

This solves the issue that the comment warns about and seems to be the appropriate way to use the loader.

I'm going to try to audit the Adapter and ContentResolver code a bit more, but if you don't hear from me, assume it's all good.

Feel free to defer this review if you want to lower your queue or know anyone better to do it.
Attachment #818168 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 818168 [details] [diff] [review]
Part 1: Do not restart Loader when thumbnails load.

Nevermind - I just realized that only the favicon thumbnails were drawing - not the screenshots.
Attachment #818168 - Flags: review?(lucasr.at.mozilla)
This seems hard to repro as of 10/17's Nightly - I believe it is due to the patch in bug 926574.

On a Galaxy Nexus (4.2.2) and a Galaxy Chat (4.0.4), I can't see any flicker. On a Motorola Droid 2 (2.3.4), I see a very fast flicker (seemingly negligible though to be fair, it's a clean profile). I wonder if the issue is just related to the Android version now.

I think it's a good idea to leave this bug open for a week or so to see if the issue returns and if not, WONTFIX it. What do you think, Aaron?
Attachment #818168 - Attachment is obsolete: true
I still see this very badly on Aurora but not Nightly, so something needs uplifting to Aurora (Fx26). If it's that bug you mentioned, I would vouch for that. Happy to close this once I can verify that it's fixed on Aurora.
I locally backed out the patch in bug 926574 and can confirm it is what caused the changes.
Depends on: 926574
verifyme on aurora and then close this out
Keywords: verifyme
Works for me on the latest Aurora (10/23).
Closing.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
Verified fixed on:
Build: Firefox for Android 26.0a2 (2013-10-27) and Firefox for Android 27.0a1 (2013-10-28)
Device: Samsung Galaxy Nexus
OS: Android 4.1.1
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.