Closed
Bug 1232968
Opened 10 years ago
Closed 10 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•10 years ago
|
tracking-fennec: --- → ?
Updated•10 years ago
|
Flags: needinfo?(liuche)
Assignee | ||
Comment 1•10 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•10 years ago
|
Assignee: nobody → liuche
Updated•10 years ago
|
Assignee | ||
Comment 2•10 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•10 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•10 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•10 years ago
|
||
...lol, what embarrassing things. Incremental replacement should always be followed by a sanity check! Thanks :)
Comment 7•10 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Reporter | ||
Comment 8•10 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•5 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
•