1010268 - Default home panel indicator is broken in settings in Fx30

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Settings and Preferences, defect)

Description

[:Margaret Leibovic]

This is fallout from bug 987175, we didn't update the PanelsPreferenceCategory logic to account for hiding the "Add panel" item:

http://hg.mozilla.org/releases/mozilla-beta/annotate/f29ab60028d4/mobile/android/base/preferences/PanelsPreferenceCategory.java#l115

Comment 1

[:Margaret Leibovic]

I'm working on properly removing this item in bug 1009586, so maybe we can just uplift that bug all the way to beta when it's ready, and that would fix this problem.

Comment 2

[:Margaret Leibovic]

Attachment: patch for beta

This bug is only an issue if the first panel is your default panel, but it's a small and easy fix. I think we should do this.

Comment 3

[Chenxia Liu [:liuche]]

Comment on attachment 8423255 [details] [diff] [review]
patch for beta

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

Looks good.

Comment 4

[Chenxia Liu [:liuche]]

Comment on attachment 8423255 [details] [diff] [review]
patch for beta

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

Oh wait, I think there's one more thing you need to change - see PanelsPreferenceCategory.refresh(). The preference removing can just be changed to removeAll().

Comment 5

[:Margaret Leibovic]

Attachment: patch for beta (v2)

Like this? I'm not sure how to test this logic path.

Comment 6

[Chenxia Liu [:liuche]]

Comment on attachment 8423751 [details] [diff] [review]
patch for beta (v2)

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

Yep! That's right.

With the previous version of the patch, moving items up or down should cause some problems because the first item in the list is never removed during a refresh().

Comment 7

[:Margaret Leibovic]

Comment on attachment 8423751 [details] [diff] [review]
patch for beta (v2)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 987175
User impact if declined: "default" indicator can be missing from home panels settings
Testing completed (on m-c, etc.): tested locally
Risk to taking this patch (and alternatives if risky): low-risk, small change to home panels settings page
String or IDL/UUID changes made by this patch: none

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e1174b7947c9

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/e1174b7947c9

Comment 10

[Teodora Vermesan (:TeoVermesan)]

Build: Firefox for Android 30 Beta 6
Device:Alcatel One Touch (Android 4.1.2) and  Nexus 4 (Android 4.4.2)
Following the steps from Bug 969060: The "default" subtext is present in the home panels settings.