1255461 - Can not set a master password

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Logins, Passwords and Form Fill, defect)

Description

[Flaviu Cos]

Environment: 
Device: Asus ZenPad 8 (Android 5.0.2);
Build: Nightly 48.0a1 (2016-03-10);

Steps to reproduce:
1. Launch Fennec;
2. Go to Settings->Privacy;
3. Set a master password.

Expected result:
The master password is successfully saved.

Actual result:
The master password is not saved.

Comment 1

[:Margaret Leibovic]

Time to kill master password? Do we know how many people are using it?

Comment 2

[:Margaret Leibovic]

Do you have logs?

Comment 3

[:Margaret Leibovic]

Also it would be helpful to get a regression window.

Comment 4

[:Margaret Leibovic]

I'll own figuring out what's going wrong here.

Comment 5

[Flaviu Cos]

Attachment: master password logs.txt

(In reply to :Margaret Leibovic from comment #2)
> Do you have logs?
Attached are the logs form logcat.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

I started using WebIDE to see if I could find a problem on the Gecko side. During the process of setting a Master Password (Settings > Dialog > OK), I never saw a breakpoint get hit in browser.js to handle the preference change.

The Android UI opens the dialog and tries to send the "Preferences:Set" message here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java#1398

Comment 7

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #6)
> I started using WebIDE to see if I could find a problem on the Gecko side.
> During the process of setting a Master Password (Settings > Dialog > OK), I
> never saw a breakpoint get hit in browser.js to handle the preference change.
> 
> The Android UI opens the dialog and tries to send the "Preferences:Set"
> message here:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/
> mozilla/gecko/preferences/GeckoPreferences.java#1398

There is no listener for "Preferences:Set" on the JS side... so that's probably why this isn't working.

This looks like it was caused by bug 1243049.

Not sure why we marked this as tracking 46, this should only affect 46.

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Use PrefsHelper to set master password (v1)

This should work.

Comment 9

[:Margaret Leibovic]

Comment on attachment 8729645 [details] [diff] [review]
Use PrefsHelper to set master password (v1)

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

Nice.

Comment 10

[Barbara Bermes [:barbara] - NI please!]

http://mzl.la/1TIgNFw??

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Barbara Bermes [:barbara] from comment #10)
> http://mzl.la/1TIgNFw??

This bug should only affect 37 and 38

Comment 12

[:Margaret Leibovic]

(In reply to Barbara Bermes [:barbara] from comment #10)
> http://mzl.la/1TIgNFw??

We don't set this flag in Fennec, so this data isn't accurate. If we want to measure this, we need to add a histogram probe somewhere, modeled after the desktop version:
http://hg.mozilla.org/mozilla-central/annotate/5e14887312d4/browser/base/content/browser.js#l1360

Want to file a bug for that?

Comment 13

[Barbara Bermes [:barbara] - NI please!]

https://bugzilla.mozilla.org/show_bug.cgi?id=1257188 done

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Notify pref handlers when pseudo-prefs change (v1)

When "pseudo-prefs" change, the Java side expects the same change
notification as regular prefs.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/071a81d2a723
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa409880028a

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/071a81d2a723
https://hg.mozilla.org/mozilla-central/rev/aa409880028a

Comment 17

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8729645 [details] [diff] [review]
Use PrefsHelper to set master password (v1)

Uplift for *both* patches.

Approval Request Comment
[Feature/regressing bug #]: Bug 1243049
[User impact if declined]: Cannot use the master password feature
[Describe test coverage new/current, TreeHerder]: Locally, m-c
[Risks and why]: Very small; patches only address this bug
[String/UUID change made/needed]: None

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Flaviu, could you please verify this issue is fixed as expected on a latest Fennec Nightly build? Thanks!

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked, recent regression in Fx47.

Comment 20

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8729645 [details] [diff] [review]
Use PrefsHelper to set master password (v1)

Recent regression, was locally verified, Aurora47+

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f9afa1afdde0
https://hg.mozilla.org/releases/mozilla-aurora/rev/ea64e619898b

Comment 22

[Flaviu Cos]

Verified as fixed in build 48.0a1 2016-03-23;
Device: Asus ZenPad 8 (Android 5.0.2).

Comment 23

[Flaviu Cos]

Verified as fixed in build 47.0a2 2016-03-24;
Device: Asus ZenPad 8 (Android 5.0.2).