Closed Bug 1029046 Opened 7 years ago Closed 7 years ago

Disable recent tabs panel in migration if all panels are disabled

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(fennec33+)

VERIFIED FIXED
Firefox 33
Tracking Status
fennec 33+ ---

People

(Reporter: Margaret, Assigned: lucasr)

References

Details

Attachments

(3 files)

This is something we discussed in bug 1004850, but which failed to make it into the migration patch.

Unfortunately, this will have already affected Nightly users who have updated, but we can fix this before this migration moves to Aurora and beyond.
tracking-fennec: ? → 33+
Actually, we would still want to add the new panel to the config, since it's a built-in panel, but we would just want to make it disabled like all the other panels.
Summary: Don't add recent tabs panel in migration if all panels are disabled → Disable recent tabs panel in migration if all panels are disabled
Attached patch WIPSplinter Review
This is kinda tricky, since we need to first iterate through all the existing panels to make sure that they're all disabled before we decide to make the new panel disabled. This shouldn't be too bad, since users generally don't have more than 10 panels (even that seems extreme), and this only runs once.

I didn't test this, but this should take us in the right direction.
Attachment #8447520 - Flags: feedback?(lucasr.at.mozilla)
Comment on attachment 8447520 [details] [diff] [review]
WIP

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

This is likely to fix bug 1030141 too because we were unconditionally adding the new built-in panel without account for the case where all existing panels were disabled (in which case the new built-in panel would have to be set as default).

::: mobile/android/base/home/HomeConfigPrefsBackend.java
@@ +150,5 @@
> +                    for (int i = 0; i < count; i++) {
> +                        final JSONObject jsonPanelConfig = originalJsonPanels.getJSONObject(i);
> +
> +                        // If any panel is enabled, then we should make the recent tabs panel enabled.
> +                        if (!jsonPanelConfig.optBoolean(JSON_KEY_DISABLED, false)) {

Does this patch even build? JSON_KEY_DISABLED is not currently accessible from the backend.
Attachment #8447520 - Flags: feedback?(lucasr.at.mozilla) → feedback+
Comment on attachment 8449425 [details] [diff] [review]
Expose HomeConfig JSON keys to other 'home' classes (r=mfinkle)

We need access to those keys in HomeConfig's backend code.
Attachment #8449425 - Flags: review?(mark.finkle)
Comment on attachment 8449426 [details] [diff] [review]
/1030141 - Conditionally enable new recent tabs panel in config migration (r=mfinkle)

Here's a more complete version of Margaret's patch. Only enable the new built-in panel if the user hasn't disabled all panels.
Attachment #8449426 - Flags: review?(mark.finkle)
Steps to verify this bug:
1. Install a nightly built before bug 1030141 landed with a fresh profile.
2. Disable all built-in panels in Settings -> Customize -> Home.
3. about:home will only show the Firefox watermark now.
3. Install new nightly with this fix included.
4. You should still see the same Firefox watermark in about:home.
Blocks: 1030141
Assignee: margaret.leibovic → lucasr.at.mozilla
No longer blocks: 1030141
Attachment #8449425 - Flags: review?(mark.finkle) → review+
Attachment #8449426 - Flags: review?(mark.finkle) → review+
(In reply to Lucas Rocha (:lucasr) from comment #8)
> Steps to verify this bug:
> 1. Install a nightly built before bug 1030141 landed with a fresh profile.

Err, I meant bug 1004850.
Blocks: 1030141
Blocks: 1031363
https://hg.mozilla.org/mozilla-central/rev/f93d3036c5a4
https://hg.mozilla.org/mozilla-central/rev/34e8a8a7603b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Blocks: 1030141
Tested with:
Device: LG Nexus 4
OS: Android 4.4.2
I have installed the 22-06 build, disabled all panels (the watermark appears in about:home), update to 04-07 build and the Firefox watermark is still present in about:home.
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.