Closed Bug 1009586 Opened 6 years ago Closed 6 years ago

Get rid of "Add panel" menu item in home settings


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

Not set



Firefox 32
Tracking Status
firefox31 --- verified
firefox32 --- verified
fennec 31+ ---


(Reporter: Margaret, Assigned: Margaret)


(Blocks 1 open bug)



(2 files, 1 obsolete file)

The home panels add-on flow has evolved since our initial designs, and I don't think the panel picker dialog makes sense anymore. Instead, I think we should use a.m.o. or some other page for users to browse add-ons that add new panels, and we can just work under the assumption that installing an add-on should install a panel.
I just realized that if we get rid of the "Add panel" menu item in settings, we wouldn't give users any way to add pack add-on panels that they removed. Perhaps instead of allowing users to remove any panels, we should only allow disabling, like we do for built-in panels. Then, to remove the panel, they could just remove the add-on.

We have add-on APIs to listen for when panels are installed/uninstalled from the home page, so add-ons could do the right thing to turn syncing on/off.
Summary: Get rid of panel picker dialog → Get rid of "Add panel" menu item in home settings
In addition to getting rid of the "Add panel" item, this patch makes dynamic panels behave the same way at built-in panels, meaning they can be hidden but not removed. If users want to remove the panel altogether, they can uninstall whatever add-on they installed.

I'm debating whether or not we should get rid of the whole panel picker activity. With this patch, it will be unused, but will we use it for something in the future? This is something we need to think about.
Attachment #8422506 - Flags: review?(liuche)
Comment on attachment 8422506 [details] [diff] [review]
Get rid of "Add panel" item in home settings

Review of attachment 8422506 [details] [diff] [review]:

Also update PanelsPreferenceCategory.refresh() to omit the special-casing of the first Preference item (see review comment in bug 1010268). Otherwise, you'll run into some problems if you try to remove/disable the first item.

::: mobile/android/base/preferences/
@@ +98,1 @@
>          // Built-in panels can't be removed, so use show/hide options.

Since we're not doing anything special about built-in panels, update this comment, or remove it entirely.
Attachment #8422506 - Flags: review?(liuche) → review+
For Fx30 we hid this menu item, but it's still present on Fx31, so we'll want to uplift this to get rid of it there.
tracking-fennec: --- → ?
Doing this in a separate patch, so we can uplift the first on to Aurora.

I'll also file a follow-up bug about removing the panel picker code if we don't see any other use for it.
Attachment #8423816 - Flags: review?(liuche)
Updated to address review comments.
Attachment #8422506 - Attachment is obsolete: true
Depends on: 1011470
I'll let this bake a bit on nightly, then request uplift.
Attachment #8423816 - Flags: review?(liuche) → review+
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Verified fixed on:
Build: Firefox for Android 32.0a1 (2014-05-18)
Device: LG Nexus 4 (Android 4.4.2)
Duplicate of this bug: 989430
Comment on attachment 8423819 [details] [diff] [review]
Get rid of "Add panel" item in home settings (v2)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new home panels APIs
User impact if declined: "Add panel" item appears in settings and confuses users
Testing completed (on m-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): low-risk, removes an item from settings
String or IDL/UUID changes made by this patch: none
Attachment #8423819 - Flags: approval-mozilla-aurora?
Attachment #8423819 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed on
Build: Firefox for Android 31.0a2 (2014-05-21)
Device: Asus Transformer Pad (Android 4.2.1)

Based on Teodora's Comment9 I will mark this bug as verified fixed.
tracking-fennec: ? → 31+
Oops, realized I never landed the second part of this:
Depends on: 1023544
You need to log in before you can comment on or make changes to this bug.