1146065 - Logins captured but not filled on discover.com

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect)

Description

[Justin Dolske [:Dolske]]

Logins on discovercard.com are captured, but not filled.

Logging output:

...
Login Manager (content): Password not filled. None of the stored logins match the username already present.
...

Adding a little extra debugging yields:

Login Manager (content): Username field value is: Password
Login Manager (content): Password field value is: 
Login Manager (content): Username field id/name is: password-shim / 
Login Manager (content): Password field id/name is: login-password / password
Login Manager (content): Password not filled. None of the stored logins match the username already present.

The password field at this point is correct. But the id of the username field we're trying to use ("password-shim") does not exist by the time I look at the page with the Inspector.

I suspect the page uses this "password-shim" as a placeholder for browsers that don't support @placeholder, and removes it at some point for browsers that do.

We should be able to fix this with a recipe to select the correct username field (either #login-account or @name=userID).

Comment 1

[Justin Dolske [:Dolske]]

(Oops, forgot this has been redirecting to discover.com for a few years! *musclememory*)

Comment 2

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

I added some extra logging in _getFormFields(), since it seems like this may be increasingly useful for recipes.

Comment 3

[Justin Dolske [:Dolske]]

Attachment: Patch v.2

Oops.

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8581220 [details] [diff] [review]
Patch v.2

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

I confirmed that this works and makes sense and I don't see any other login forms on that domain that wouldn't want this recipe.

::: toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ +456,5 @@
>      if (!usernameField)
>        log("(form -- no username field found)");
>  
> +    log("Username field id/name/value is: " + usernameField.id + " / " +
> +        usernameField.name + " / " + usernameField.value);

Is there a reason you used string concatenation here instead of having that done in `log` (via rest arguments)?

::: toolkit/components/passwordmgr/LoginRecipes.jsm
@@ +240,5 @@
> +    {
> +      "description": "An ephemeral password-shim field is incorrectly selected as the username field.",
> +      "hosts": ["www.discover.com"],
> +      "usernameSelector": "#login-account"
> +    },

Can you remove the trailing comma here so this is valid JSON so we can `hg copy` it as-is to a JSON file in bug 1134850. (Not a huge deal since this isn't on an important line and we can fix it in that bug too).

Comment 5

[Justin Dolske [:Dolske]]

Fixed nits and landed.

https://hg.mozilla.org/integration/fx-team/rev/4874b0778b68

Comment 6

[Justin Dolske [:Dolske]]

...and a bustage fix.

https://hg.mozilla.org/integration/fx-team/rev/1e5dcedd40cc

Comment 7

[Mark Hammond [:markh] [:mhammond]]

https://hg.mozilla.org/mozilla-central/rev/4874b0778b68
https://hg.mozilla.org/mozilla-central/rev/1e5dcedd40cc