Closed Bug 1025812 Opened 6 years ago Closed 6 years ago

Last added pin to Top Sites Grid has the name grayed out

Categories

(Firefox for Android :: Awesomescreen, defect)

32 Branch
All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 33
Tracking Status
firefox31 --- unaffected
firefox32 + verified
firefox33 + verified
fennec 32+ ---

People

(Reporter: ioana.chiorean, Assigned: lucasr)

References

Details

(Whiteboard: regression)

Attachments

(3 files)

Attached image Screenshot
OS: Android 4.4 
Device: Nexus 4

From bug 1025062 we had that at the last added site to top sites grid you will have the name grayed out till u have a full pinned grid. 

while searching a regression for that bug I found the one for this :
last good: 05/07
first bad: 05/08
Pushlog:http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5bbc85136202&tochange=2a03b34c8953
Either bug 997660 or bug 996657.
tracking-fennec: --- → ?
Assignee: nobody → lucasr.at.mozilla
tracking-fennec: ? → 32+
Comment on attachment 8451081 [details] [diff] [review]
Part 1: Factor out type updates into separate method (r=rnewman)

Update mType and compound drawables in a single spot. Prep work for the actual fix.
Attachment #8451081 - Flags: review?(rnewman)
Comment on attachment 8451082 [details] [diff] [review]
Part 2: Refresh TopSitesGridItemView's drawable state when type changes (r=rnewman)

In bug 997660, I changed the grid item's drawable state to depend on its type instead of the URL/title. So, we must refresh the drawable state whenever the type changes.
Attachment #8451082 - Flags: review?(rnewman)
Status: NEW → ASSIGNED
OS: All → Android
Hardware: ARM → All
Summary: Last added pin to Top Sited Grid has the name grayed out → Last added pin to Top Sites Grid has the name grayed out
Attachment #8451081 - Flags: review?(rnewman) → review+
Comment on attachment 8451082 [details] [diff] [review]
Part 2: Refresh TopSitesGridItemView's drawable state when type changes (r=rnewman)

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

::: mobile/android/base/home/TopSitesGridItemView.java
@@ +294,5 @@
>              return false;
>          }
>  
>          mType = type;
> +        refreshDrawableState();

Is there a good reason to do this here, rather than after futzing with mTitleView on line 301?
Attachment #8451082 - Flags: review?(rnewman) → review+
(In reply to Richard Newman [:rnewman] from comment #6)
> Comment on attachment 8451082 [details] [diff] [review]
> Part 2: Refresh TopSitesGridItemView's drawable state when type changes
> (r=rnewman)
> 
> Review of attachment 8451082 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/home/TopSitesGridItemView.java
> @@ +294,5 @@
> >              return false;
> >          }
> >  
> >          mType = type;
> > +        refreshDrawableState();
> 
> Is there a good reason to do this here, rather than after futzing with
> mTitleView on line 301?

Doing this whenever the type changes is more future-proof because the drawable state only depends on type now.
Comment on attachment 8451081 [details] [diff] [review]
Part 1: Factor out type updates into separate method (r=rnewman)

Approval Request Comment
[Feature/regressing bug #]: bug 988366
[User impact if declined]: Top sites grid will show items in an incorrect state i.e. greyed out title for normal grid cells.   
[Describe test coverage new/current, TBPL]: Local tests only. Let's bake this in Nightly for a couple days and then uplift.
[Risks and why]: Low, just changes the code to update the drawable state of each grid item.
[String/UUID change made/needed]: n/a
Attachment #8451081 - Flags: approval-mozilla-aurora?
Comment on attachment 8451082 [details] [diff] [review]
Part 2: Refresh TopSitesGridItemView's drawable state when type changes (r=rnewman)

Approval Request Comment
[Feature/regressing bug #]: bug 988366
[User impact if declined]: Top sites grid will show items in an incorrect state i.e. greyed out title for normal grid cells.   
[Describe test coverage new/current, TBPL]: Local tests only. Let's bake this in Nightly for a couple days and then uplift.
[Risks and why]: Low, just changes the code to update the drawable state of each grid item.
[String/UUID change made/needed]: n/a
Attachment #8451082 - Flags: approval-mozilla-aurora?
Verified as fixed in build:
Nightly 33.0a1 (2014-07-09)

Devices:
Motorola Razr (Android 4.0.4)
Galaxy Nexus (Android 4.2.1)
Comment on attachment 8451081 [details] [diff] [review]
Part 1: Factor out type updates into separate method (r=rnewman)

Aurora+
Attachment #8451081 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8451082 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed in:
Firefox for Android 32.0a2 (2014-07-13)
Device:
Galaxy Nexus (Android 4.2.1)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.