Closed Bug 1146065 Opened 5 years ago Closed 5 years ago

Logins captured but not filled on discover.com

Categories

(Toolkit :: Password Manager, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: Dolske, Assigned: Dolske)

References

Details

Attachments

(1 file, 1 obsolete file)

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).
(Oops, forgot this has been redirecting to discover.com for a few years! *musclememory*)
Summary: Logins captured but not filled on discovercard.com → Logins captured but not filled on discover.com
Attached patch Patch v.1 (obsolete) — Splinter Review
I added some extra logging in _getFormFields(), since it seems like this may be increasingly useful for recipes.
Assignee: nobody → dolske
Attachment #8581219 - Flags: review?(MattN+bmo)
Attached patch Patch v.2Splinter Review
Oops.
Attachment #8581219 - Attachment is obsolete: true
Attachment #8581219 - Flags: review?(MattN+bmo)
Attachment #8581220 - Flags: review?(MattN+bmo)
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).
Attachment #8581220 - Flags: review?(MattN+bmo) → review+
Depends on: 1211780
You need to log in before you can comment on or make changes to this bug.