Last Comment Bug 769896 - Make "Import from Android" use MultiChoicePreference
: Make "Import from Android" use MultiChoicePreference
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
: -- enhancement (vote)
: Firefox 16
Assigned To: Gian-Carlo Pascutto [:gcp]
:
Mentors:
: 770355 (view as bug list)
Depends on: 710330 754335
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-30 01:21 PDT by Gian-Carlo Pascutto [:gcp]
Modified: 2012-07-26 11:50 PDT (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
fixed
verified
verified


Attachments
Patch 1. Use MultiChoicePreference (6.47 KB, patch)
2012-07-12 07:57 PDT, Gian-Carlo Pascutto [:gcp]
bnicholson: review+
akeybl: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Gian-Carlo Pascutto [:gcp] 2012-06-30 01:21:28 PDT
Bug 754335 added a MultiChoicePreference dialog. The "Import from Android" in bug 710330 already had a lightweight version of that. We should change the latter to use the former.
Comment 1 Gian-Carlo Pascutto [:gcp] 2012-07-04 11:15:46 PDT
*** Bug 770355 has been marked as a duplicate of this bug. ***
Comment 2 Gian-Carlo Pascutto [:gcp] 2012-07-12 07:57:20 PDT
Created attachment 641466 [details] [diff] [review]
Patch 1. Use MultiChoicePreference
Comment 3 Brian Nicholson (:bnicholson) (PTO through August 1) 2012-07-12 11:14:25 PDT
Comment on attachment 641466 [details] [diff] [review]
Patch 1. Use MultiChoicePreference

Too bad this seems to just complicate your code more. Maybe MultiChoicePreference should return a Map instead of different arrays.
Comment 4 Gian-Carlo Pascutto [:gcp] 2012-07-12 11:45:24 PDT
More functionality for approximately the same amount of code isn't much of a complication :)

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f4ad785cca8
Comment 5 Gian-Carlo Pascutto [:gcp] 2012-07-12 11:47:09 PDT
Comment on attachment 641466 [details] [diff] [review]
Patch 1. Use MultiChoicePreference

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 710330
User impact if declined: Settings dialog lost on rotation
Testing completed (on m-c, etc.): Just landed on m-c
Risk to taking this patch (and alternatives if risky): Broken UI? Low risk.
Comment 6 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-12 17:35:32 PDT
(In reply to Gian-Carlo Pascutto (:gcp) from comment #5)
> Comment on attachment 641466 [details] [diff] [review]
> Patch 1. Use MultiChoicePreference
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): Bug 710330
> User impact if declined: Settings dialog lost on rotation
> Testing completed (on m-c, etc.): Just landed on m-c
> Risk to taking this patch (and alternatives if risky): Broken UI? Low risk.

So does this have string changes then?
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-07-12 18:03:51 PDT
https://hg.mozilla.org/mozilla-central/rev/1f4ad785cca8
Comment 8 Gian-Carlo Pascutto [:gcp] 2012-07-12 21:49:03 PDT
>So does this have string changes then?

No, it's a functional change in existing UI.
Comment 9 Gian-Carlo Pascutto [:gcp] 2012-07-25 08:48:12 PDT
Comment on attachment 641466 [details] [diff] [review]
Patch 1. Use MultiChoicePreference

[Approval Request Comment]
Same as for the approval-aurora that already got a +. I forgot to land this patch on Aurora.
Comment 10 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-25 10:28:38 PDT
Comment on attachment 641466 [details] [diff] [review]
Patch 1. Use MultiChoicePreference

Mobile only, so approving for Beta landing.
Comment 11 Gian-Carlo Pascutto [:gcp] 2012-07-26 00:26:22 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/62a1df5fee45
Comment 12 Aaron Train [:aaronmt] 2012-07-26 11:38:57 PDT
aurora?
Comment 13 Gian-Carlo Pascutto [:gcp] 2012-07-26 11:47:02 PDT
It's already in Aurora. The patch landed in Nightly, and didn't get in Aurora before the migration date (16th July), so it's in Aurora now because it was in Nightly, and was missing in Beta because it wasn't in Aurora (until comment 11 added it to beta after the migration).

Note You need to log in before you can comment on or make changes to this bug.