Closed Bug 700527 Opened 8 years ago Closed 8 years ago

Master password prompt has 4 characters pre-populated

Categories

(Firefox for Android :: General, defect, P2)

defect

Tracking

()

VERIFIED FIXED
Firefox 12
Tracking Status
firefox11 --- verified
firefox12 --- verified
fennec 11+ ---

People

(Reporter: catlee, Assigned: wesj)

References

Details

Attachments

(1 file)

When the master password prompt pops up, it has 4 characters already present in the password field. No idea what they are, but I bet they shouldn't be there!
Product: Fennec → Fennec Native
Version: Trunk → unspecified
Master password is not part of Native at this time. You must be using XUL nightly.
Product: Fennec Native → Fennec
Version: unspecified → Trunk
about:buildconfig says I'm running a birch build.
I'm using a master password on native.  I'll confirm this.  There's most certain a pref for it in the preferences screen, and it does work.
Product: Fennec → Fennec Native
Version: Trunk → unspecified
Which preference?

'Remember Passwords' ≠ 'Master Password'
Hmm, you're right, I don't see a preference for it.  It's most certainly prompting me though.  Maybe it remembers my preference from before I upgraded from the XUL version?
(In reply to Dave Miller [:justdave] from comment #6)
> Hmm, you're right, I don't see a preference for it.  It's most certainly
> prompting me though.  Maybe it remembers my preference from before I
> upgraded from the XUL version?

Yep. Bug 704547
Priority: -- → P5
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 704547
I am reopening this bug. We can't just reset the master password. It would strand all the existing passwords.
Status: RESOLVED → REOPENED
Priority: P5 → P2
Resolution: DUPLICATE → ---
Attached patch PatchSplinter Review
The real bug here is just a dumb mistake in PromptService.js that sends "null" as the value for the password box.

BUT! This gives us master password support in native Fennec. I wanted to use the normal PromptService we have for that as well, but it (currently?) assumes we're in the GeckoApp context, and shows dialogs on the GeckoApp. I'm not sure how to know from GeckoAppShell if we're showing the prefs pane and then instead use its context. Just decided it would be easier in this case to use the native dialog management. Plus, then we can do the cool "disable the ok button if your passwords don't match" trick.
Assignee: nobody → wjohnston
Attachment #583289 - Flags: review?(mark.finkle)
Comment on attachment 583289 [details] [diff] [review]
Patch

>diff --git a/mobile/android/base/GeckoPreferences.java b/mobile/android/base/GeckoPreferences.java
>+    protected Dialog onCreateDialog(int id) {

>+            case DIALOG_CREATE_MASTER_PASSWORD:
>+                // do the work to define the pause Dialog

Wrong comment ("pause dialog"?)

>+            case DIALOG_REMOVE_MASTER_PASSWORD:
>+                // do the work to define the game over Dialog

Wrong comment

>diff --git a/mobile/android/base/locales/en-US/android_strings.dtd b/mobile/android/base/locales/en-US/android_strings.dtd

>+<!ENTITY masterpassword_create_title "Create master password">
>+<!ENTITY masterpassword_remove_title "Remove master password">

Use title case for these:
"Create Master Password"
"Remove Master Password"

>+<!ENTITY masterpassword_retype "Retype password">

We used "Confirm password" in XUL, which could really be shortened to just "Confirm" imo. What do you think?

>+<!ENTITY masterpassword_oldpassword "Old password">

Just use masterpassword_password for this case? ("Password")

>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

>+      if (MasterPassword.enabled) {
>+        MasterPassword.removePassword(json.value);
>+      } else {
>+        MasterPassword.setPassword(json.value);
>+      }

{} not needed

>+var MasterPassword = {

>+  setPassword: function setPassword(password) {

aPassword

>+      if (status == Ci.nsIPKCS11Slot.SLOT_UNINITIALIZED) {
>+        token.initPassword(password);
>+      } else if (status == Ci.nsIPKCS11Slot.SLOT_READY) {
>+        token.changePassword("", password);
>+      }

{} not needed

>+    } catch(e) {
>+      dump("--- MasterPasswordUI._setPassword exception: " + e + "\n");

        dump("MasterPassword.setPassword: " + e);

>+  removePassword: function removePassword(aOldPassword) {

>+    } catch(e) {
>+      dump("--- MasterPasswordUI._removePassword exception: " + e + "\n");

        dump("MasterPassword.removePassword: " + e);

>+    }
>+    NativeWindow.toast.show(Strings.browser.GetStringFromName("masterPassword.incorrect"), "short");

Add a line break before the NativeWindow


r+ but fix the nits
Attachment #583289 - Flags: review?(mark.finkle) → review+
Whiteboard: [fennec-aurora]
landed on inbound. i'll push to aurora if that looks ok

https://hg.mozilla.org/integration/mozilla-inbound/rev/1b960b4cc99f
Group: mozilla-confidential
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Group: mozilla-confidential
Depends on: 713600
Comment on attachment 583289 [details] [diff] [review]
Patch

We can't leave users who have Master Password enabled from Fx9 or Fx10 be stranded when Fx11 is released. This patch adds simple support for Master Password to Fennec Native.
Attachment #583289 - Flags: approval-mozilla-aurora?
Comment on attachment 583289 [details] [diff] [review]
Patch

[triage comment]
Approved for aurora. Mobile-only, needed to not "strand" XUL -> native users with a master password set.
Attachment #583289 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [fennec-aurora]
Flags: in-litmus?(fennec)
Whiteboard: [QA+]
Verified with:
Mozilla/5.0 (Android; Linux armv7l; rv:12.0a1) Gecko/20111229 Firefox/12.0a1
Fennec/12.0a1
Samsung Galaxy S2 (Android 2.3)

Mozilla/5.0 (Android; Linux armv7l; rv:11.0a2) Gecko/20111229 Firefox/11.0a2
Fennec/11.0a2
Samsung Galaxy S2 (Android 2.3)

Characters are not pre-populated in master password field.
(Also tried the scenario when upgrading from an XUL build to native fennec with master password enabled.)
Status: RESOLVED → VERIFIED
Whiteboard: [QA+]
tracking-fennec: --- → 11+
Testcase created for the upgrade scenario.
https://litmus.mozilla.org/show_test.cgi?id=45451
Flags: in-litmus?(fennec) → in-litmus+
You need to log in before you can comment on or make changes to this bug.