Closed Bug 962606 Opened 10 years ago Closed 10 years ago

Firefox Accounts (Sign-Up) - Suggestions are present for password field

Categories

(Firefox for Android Graveyard :: Android Sync, defect, P1)

ARM
Android
defect

Tracking

(firefox29 verified, firefox30 verified, fennec29+)

VERIFIED FIXED
Tracking Status
firefox29 --- verified
firefox30 --- verified
fennec 29+ ---

People

(Reporter: ioana.chiorean, Assigned: blassey)

Details

(Whiteboard: [qa+][parallel])

Attachments

(2 files, 1 obsolete file)

Attached image password.jpg
Build: 2014-01-22 Nightly Firefox 29
OS: Android 4.2.2
Device: Galaxy Nexus 

Precondition: Have keyboard prediction on 

Steps:
1. Settings ->  -> Add account -> Firefox Account
2. Configure a new account
3. While in password field tap show
4. Tap Hide
5. Tap Show again

Expected result:
- There should be no suggestions for password

Actual result:
- Suggestions are given for the text you enter.; After step 5 the suggestion line is empty.
Old Sync

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/setup/activities/AccountActivity.java#79

The EditText from GeckoPreferences

inputtype |= InputType.TYPE_TEXT_VARIATION_PASSWORD | InputType.TYPE_TEXT_FLAG_NO_SUGGESTIONS;
tracking-fennec: --- → ?
tracking-fennec: ? → 29+
Priority: -- → P1
Whiteboard: [qa+][parallel]
Jim, sounds like the keyboard interaction is getting wonky, can you have a look?
Assignee: nobody → nchen
Wait, isn't it more likely we just haven't configured the password field correctly in the layout XML?
(In reply to Nick Alexander :nalexander from comment #3)
> Wait, isn't it more likely we just haven't configured the password field
> correctly in the layout XML?

that was my first though, but I assume the show->hide->show implies that we're not setting some attribute right on that action
Assignee: nchen → blassey.bugs
Attached patch sync_password_suggest.patch (obsolete) — Splinter Review
Attachment #8381567 - Flags: review?(nalexander)
Comment on attachment 8381567 [details] [diff] [review]
sync_password_suggest.patch

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

Fine, with nits.  This needs to land upstream, in android-sync, though -- probably easiest for me just to address nits and do that.  (Unless you plan to contribute to FxA more -- which would be great; then you should do that.)  Will update the ticket with landing info if I don't hear otherwise.

::: mobile/android/base/fxa/activities/FxAccountAbstractSetupActivity.java
@@ +63,4 @@
>  
>    protected void createShowPasswordButton() {
>      showPasswordButton.setOnClickListener(new OnClickListener() {
> +      boolean isShown = false;

nit: private, and new line following.

Can this be determined from |passwordEdit| itself, rather than hanging state on a (possibly transient) listener?

@@ +74,2 @@
>          if (isShown) {
> +          // hide password

nit: all comments full sentences.

@@ +74,3 @@
>          if (isShown) {
> +          // hide password
> +          passwordEdit.setTransformationMethod(PasswordTransformationMethod.getInstance());

I remember seeing these different approaches and choosing the input type, but now I can't recall why.  If this works, fine by me.
Attachment #8381567 - Flags: review?(nalexander) → review+
Attached file 54a03ed40ad9
[Approval Request Comment]

This is a good candidate for uplift.  It needs to bake for a few days, but we know we want it, and it would be nice to uplift just to m-a rather than all the way to m-b, so opening the approval request early.

Bug caused by (feature/regressing bug #): initial FxA landing.

User impact if declined: Password show/hide button is bad on many (most?) phones.

Testing completed (on m-c, etc.): almost none.  Needs to bake for a few days.

Risk to taking this patch (and alternatives if risky): very low.

String or IDL/UUID changes made by this patch: none.
Attachment #8381567 - Attachment is obsolete: true
Attachment #8384951 - Flags: review+
Attachment #8384951 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/54a03ed40ad9
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 8384951 [details]
54a03ed40ad9

Baked for 2 days. I guess that is now enough!
Attachment #8384951 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed on:

Builds:
29.0.1
30.0b4

Devices:
Motorola Razr (Android 4.0.4)
Asus Transformer Pad TF300T (Android 4.2.1)
Status: RESOLVED → VERIFIED
Product: Android Background Services → Firefox for Android
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: