Closed Bug 939060 Opened 6 years ago Closed 6 years ago

Incorrect thumbnail is sometimes displayed on the last empty grid position

Categories

(Firefox for Android :: Awesomescreen, defect)

All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 28
Tracking Status
firefox27 --- verified
firefox28 --- verified

People

(Reporter: cos_flaviu, Assigned: lucasr)

References

Details

Attachments

(2 files)

Environment:
Build: Nightly 28.0a1 2013-11-14;
Device: HTC Desire(Android 2.2).

Steps to reproduce:
1. Start with a clean profile or create one if needed;
2. Start the app.

Expected results:
In the Top Sites grid only the first 2 positions are displayed.

Actual results:
The last grid position which is empty display the thumbnail for firefox support.

Notes:
Please check the screen capture attached.
This is a follow-on from the partial solution in Bug 926430. There's still some kind of racing view recycling going on.

My hope is that this simply disappears as we make improvements to the thumbnail and favicon loading code -- e.g., see Bug 938141 Comment 4. 

Tweaking the subject to indicate that (a) on some devices/versions this isn't readily reproducible, and (b) it's only Firefox Support because that's one of our pre-loaded bookmarks.
Hardware: ARM → All
Summary: Firefox support thumbnail is displayed on the last empty grid position → Incorrect thumbnail is sometimes displayed on the last empty grid position
Comment on attachment 8334494 [details] [diff] [review]
Check URL associated with grid item before displaying favicon (r=rnewman)

This is likely caused by the fact that we don't check if the target grid item has been bound to another URL before displaying the favicon.
Attachment #8334494 - Flags: review?(rnewman)
Assignee: nobody → lucasr.at.mozilla
Comment on attachment 8334494 [details] [diff] [review]
Check URL associated with grid item before displaying favicon (r=rnewman)

Review of attachment 8334494 [details] [diff] [review]:
-----------------------------------------------------------------

The loadID check is supposed to avoid the need for this kind of thing, but I suppose there is a window in which this could occur, which would explain the bug.

This looks like a mirror of Bug 920331, so if this fixes the problem we should make a note in that bug that there are two places to change.

Time to test this patch on my repro-ing device…
Comment on attachment 8334494 [details] [diff] [review]
Check URL associated with grid item before displaying favicon (r=rnewman)

This looks good to me. I'll take care of landing this later today, 'cos lucasr should be asleep :D
Attachment #8334494 - Flags: review?(rnewman) → review+
https://hg.mozilla.org/integration/fx-team/rev/faaaafbf6626
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 28
Comment on attachment 8334494 [details] [diff] [review]
Check URL associated with grid item before displaying favicon (r=rnewman)

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
  Old stuff; partial fix in Bug 926430. This completes it.
 
User impact if declined: 
  First-run user experience shows the wrong favicon in the Top Sites view on some devices. (Timing-related.)

Testing completed (on m-c, etc.): 
  Manually tested. Just about to land on m-c for more verification.

Risk to taking this patch (and alternatives if risky): 
  None.

String or IDL/UUID changes made by this patch:
  None.
Attachment #8334494 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/faaaafbf6626
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Flags: needinfo?(flaviu.cos)
Keywords: verifyme
Attachment #8334494 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed in build:
- Aurora 27.0a2 (2013-11-24);
- Nightly 28.0a1 (2013-11-24);
Device: HTC Desire (Android 2.2)
Status: RESOLVED → VERIFIED
Flags: needinfo?(flaviu.cos)
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.