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

Status: VERIFIED FIXED

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

Description

[Ioana Chiorean]

Attachment: 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.

Comment 1

[Aaron Train [:aaronmt]]

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;

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Jim, sounds like the keyboard interaction is getting wonky, can you have a look?

Comment 3

[Nick Alexander :nalexander [he/him]]

Wait, isn't it more likely we just haven't configured the password field correctly in the layout XML?

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

(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

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: sync_password_suggest.patch

Comment 6

[Nick Alexander :nalexander [he/him]]

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.

Comment 7

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/54a03ed40ad9

Comment 8

[Nick Alexander :nalexander [he/him]]

Attachment: 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.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/54a03ed40ad9

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8384951 [details]
54a03ed40ad9

Baked for 2 days. I guess that is now enough!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ae3070a45dc5

Comment 12

[Cristina Madaras, QA [:CristinaM]]

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)