Follow-up: Migrate users with customized panels to combined history panel

RESOLVED FIXED in Firefox 48

Status

()

Firefox for Android
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: liuche, Assigned: liuche)

Tracking

Trunk
Firefox 48
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Migrate users with custom home panels that contain History or Synced panels to the Combined History panel, and remove all the other code that we no longer need.
No longer depends on: 1260320
Created attachment 8738233 [details]
MozReview Request: Bug 1260321 - Follow-up: Migrate users with customized panels to combined history panel. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/44377/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44377/
Attachment #8738233 - Flags: review?(s.kaspari)
I tested this with no panels enabled, just history enabled, just sync enabled by upgrading from an older version that didn't have the new sync panels.

I built from the changeset before combined history/synced panels landed: https://hg.mozilla.org/integration/fx-team/rev/69f271e79c56

(the build I used is http://people.mozilla.org/~liuche/bug-1260321/old-panels.apk )

Updated

2 years ago
Assignee: nobody → liuche
Comment on attachment 8738233 [details]
MozReview Request: Bug 1260321 - Follow-up: Migrate users with customized panels to combined history panel. r=sebastian

https://reviewboard.mozilla.org/r/44377/#review41691

::: mobile/android/base/java/org/mozilla/gecko/home/HomeConfigPrefsBackend.java:178
(Diff revision 1)
> +        if (historyIndex == -1 || syncIndex == -1) {
> +            throw new IllegalArgumentException("Missing default panels");
> +        }

Can we assume that this is never going to happen and we do not have any index bugs? Otherwise we could throw JSONException and fall back to the default config instead of crashing? But then again if we don't see this crash while this rides the trains then everything should be okay.
Attachment #8738233 - Flags: review?(s.kaspari) → review+

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fef9283ad53a
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
No longer blocks: 1262343
You need to log in before you can comment on or make changes to this bug.