top sites section of about:home resizes when thumbnails populate

VERIFIED FIXED in Firefox 11

Status

()

Firefox for Android
General
P3
normal
VERIFIED FIXED
6 years ago
11 months ago

People

(Reporter: blassey, Assigned: blassey)

Tracking

unspecified
Firefox 12
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox11 fixed, firefox12 verified, fennec11+)

Details

Attachments

(1 attachment)

Created attachment 583406 [details] [diff] [review]
patch

I am pretty sure there is already a bug filed on this, but I can't find it
Attachment #583406 - Flags: review?(mark.finkle)

Updated

6 years ago
Priority: -- → P3
Comment on attachment 583406 [details] [diff] [review]
patch


>diff --git a/mobile/android/base/AboutHomeContent.java b/mobile/android/base/AboutHomeContent.java

>     public static class TopSitesGridView extends GridView {
>+        private static final int kTopSiteItemHeight = 160;
>+

This is the only part I am worried about. Can we find a different way to get the single item height? Can we get the actual height and add a constant margin?

I suppose we could do that in a followup bug. Please file one.
Attachment #583406 - Flags: review?(mark.finkle) → review+
pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/d7999840fac5

(In reply to Mark Finkle (:mfinkle) from comment #1)
> Comment on attachment 583406 [details] [diff] [review]
> patch
> 
> 
> >diff --git a/mobile/android/base/AboutHomeContent.java b/mobile/android/base/AboutHomeContent.java
> 
> >     public static class TopSitesGridView extends GridView {
> >+        private static final int kTopSiteItemHeight = 160;
> >+
> 
> This is the only part I am worried about. Can we find a different way to get
> the single item height? Can we get the actual height and add a constant
> margin?
We can't get the actual ("measured" in Android-ese) until the element is constructed, which is what the code was doing. The problem is that these elements take a while to construct because of the DB access.

The idea here is to pre-calculate it ourselves. I changed the code to hard code the 109dip height of the tiles and apply the display density scale to that. I also added a comment showing my math to arrive at 109dip.
Whiteboard: [inbound]
(In reply to Brad Lassey [:blassey] from comment #2)

> The idea here is to pre-calculate it ourselves. I changed the code to hard
> code the 109dip height of the tiles and apply the display density scale to
> that. I also added a comment showing my math to arrive at 109dip.

That sounds good to me. No need for any followup bugs.
I did notice you are using "dm.density" and we have a GeckoAppShell.getDpi() that returns "dm.densityDpi":
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoAppShell.java#1015

I'm not 100% sure of the difference, but we do use GeckoAppShell.getDpi() in another place where we scale a dip size.
density = desityDPI / 160
https://hg.mozilla.org/mozilla-central/rev/d7999840fac5

Do we want this on Aurora?
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → Firefox 12
(Assignee)

Updated

6 years ago
Duplicate of this bug: 710327
Samsung Nexus S (Android 4.0.3)
Mozilla/5.0 (Android; Linux armv7l; rv:12.0a1) Gecko/20111230 Firefox/12.0a1 Fennec/12.0a1
Status: RESOLVED → VERIFIED
status-firefox12: --- → verified
OS: Mac OS X → Android
Hardware: x86 → ARM
(Assignee)

Updated

6 years ago
tracking-fennec: --- → 11+
(Assignee)

Updated

6 years ago
status-firefox11: --- → affected
Comment on attachment 583406 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
Attachment #583406 - Flags: approval-mozilla-aurora?
Comment on attachment 583406 [details] [diff] [review]
patch

[Triage Comment]
Mobile only - approving for Aurora.
Attachment #583406 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee: nobody → blassey.bugs
https://hg.mozilla.org/releases/mozilla-aurora/rev/1e8584b9d3b1
status-firefox11: affected → fixed
You need to log in before you can comment on or make changes to this bug.