Closed
Bug 1232968
Opened 9 years ago
Closed 8 years ago
Choosing to import only one option from onboarding screen will import both bookmarks and history
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox46 verified, fennec45+)
RESOLVED
FIXED
Firefox 46
People
(Reporter: TeoVermesan, Assigned: liuche)
References
Details
(Keywords: regression)
Attachments
(1 file)
Steps to reproduce: 1. With a clean profile open Firefox 2. From the onboarding screen choose "Transfer to Firefox" 3. Uncheck "History" option and leave only "Bookmarks" checked 4. Click "Transfer" -> Next -> Start Browsing Expected results: - History panel is empty. Bookmarks panel is populated with bookmarks from default browser Actual results: - Both History and Bookmarks panels are populated from the default browser note: - reproducible on 45.0a2 Aurora (2015-12-15), 46.0a1 Nightly (2015-12-15) - not reproducible on 43.0, 43b10 and 44.0a2 (2015-12-08)
Updated•9 years ago
|
tracking-fennec: --- → ?
Updated•9 years ago
|
Flags: needinfo?(liuche)
Assignee | ||
Comment 1•8 years ago
|
||
Okay, it looks like there's a regression from bug 1209539 - setting another listener seems to have disabled the onMultiClickListener. I'll look into this.
Flags: needinfo?(liuche)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → liuche
Updated•8 years ago
|
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28435/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/28435/
Attachment #8699762 -
Flags: review?(s.kaspari)
Assignee | ||
Comment 3•8 years ago
|
||
The OnMultiChoiceClickListener we originally set is being replaced by the listener we're later setting on the ListView [1]. I didn't check to see if this roundabout listener-setting was replacing one of our existing listeners (which it is). Also cleared up some of the checkbox state. [1] http://androidxref.com/5.1.1_r6/xref/frameworks/base/core/java/com/android/internal/app/AlertController.java#1078
Comment 4•8 years ago
|
||
Comment on attachment 8699762 [details] MozReview Request: Bug 1232968 - Choosing to import only one option from onboarding screen will import both bookmarks and history. r=sebastian https://reviewboard.mozilla.org/r/28435/#review25369 ::: mobile/android/base/java/org/mozilla/gecko/firstrun/ImportPanel.java:53 (Diff revision 1) > - final List<Integer> checked = new ArrayList<>(Arrays.asList(0, 1)); > + final Boolean[] checked = new Boolean[2]; > + Arrays.fill(checked, true); You don't need Boolean objects here, I think? (Capitalized 'b'). This will avoid Autoboxing/Unboxing[1] overhead. A compact version would be: boolean[] checked = {true, true}; [1] https://docs.oracle.com/javase/tutorial/java/data/autoboxing.html ::: mobile/android/base/java/org/mozilla/gecko/firstrun/ImportPanel.java:95 (Diff revision 1) > + final boolean isChecked = ((CheckedTextView) view).isChecked(); > + if (isChecked) { > + checked[i] = true; > + } else { > + checked[i] = false; > + } This looks like it could be simplified: checked[i] = ((CheckedTextView) view).isChecked();
Attachment #8699762 -
Flags: review?(s.kaspari) → review+
Assignee | ||
Comment 5•8 years ago
|
||
...lol, what embarrassing things. Incremental replacement should always be followed by a sanity check! Thanks :)
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/36aa441232b7
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Reporter | ||
Comment 8•8 years ago
|
||
Choosing to import: -only "History" and leave "Bookmarks" unchecked => History panel is populated. Bookmarks panel has only the default ones. -only "Bookmarks" and leave "History" unchecked => Bookmarks panel is populated. History panel is empty. Verified as fixed using: Device: Samsung S6 Edge (Android 5.1.1) Build: Firefox for Android 46.0a1 (2015-12-22)
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•