Closed
Bug 1330829
Opened 8 years ago
Closed 8 years ago
Login Manager recipe to support ignoring specific password fields
Categories
(Toolkit :: Password Manager, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: MattN, Assigned: MattN)
References
(Blocks 1 open bug)
Details
(Whiteboard: [passwords:recipes])
Attachments
(1 file)
Some sites use useless type=password fields to throw off password manager, other sites use type=password fields for things that aren't password fields e.g. credit cards or one-time use pins which the user doesn't want to save in password manager. I propose adding a notPasswordSelector recipe option that works like notUsernameSelector to filter out these fields we don't want to consider.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•8 years ago
|
||
mozreview-review |
Comment on attachment 8826425 [details] Bug 1330829 - Login Manager recipe support for ignoring specific password fields. https://reviewboard.mozilla.org/r/104368/#review105134 Seems pretty straightforward. dholbert has tried it out (https://bugzilla.mozilla.org/show_bug.cgi?id=1330810#c6) and I'll rely on him to know that this works in practice. ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:592 (Diff revision 2) > * @param {FormLike} form - the FormLike to look for password fields in. > - * @param {bool} [skipEmptyFields=false] - Whether to ignore password fields with no value. > - * Used at capture time since saving empty values isn't > - * useful. > + * @param {Object} options > + * @param {bool} [options.skipEmptyFields=false] - Whether to ignore password fields with no value. > + * Used at capture time since saving empty values isn't > + * useful. > + * @param {Object} [options.fieldOverrideRecipe=null] - A relevant field override recipe to use. I really like that options documentation style. We should use it in more places. ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:614 (Diff revision 2) > > + // Exclude ones matching a `notPasswordSelector`, if specified. > + if (fieldOverrideRecipe && fieldOverrideRecipe.notPasswordSelector && > + element.matches(fieldOverrideRecipe.notPasswordSelector)) { > + log("skipping password field (id/name is", element.id, " / ", > + element.name + ") due to recipe:", fieldOverrideRecipe); nit: The combination of "," and "+" concatenating looks a bit random here, can't you replace the single "+" with a ","? ::: toolkit/components/passwordmgr/LoginRecipes.jsm:17 (Diff revision 2) > + "description", > + "notPasswordSelector", > + "notUsernameSelector", > + "passwordSelector", > + "pathRegex", > + "usernameSelector" nit: Since you've added trailing commas in most other parts of your patch, I assume you forgot to add one here :) ::: toolkit/components/passwordmgr/test/mochitest/test_recipe_login_fields.html:141 (Diff revision 2) > +add_task(function* loadNotPasswordSelectorRecipes() { > + yield resetRecipes(); > + yield loadRecipes({ > + siteRecipes: [{ > + hosts: ["mochi.test:8888"], > + notPasswordSelector: "input[name='not_pword'], input[name='not_pword2']" nit: trailing comma
Attachment #8826425 -
Flags: review?(jhofmann) → review+
Assignee | ||
Comment 4•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8826425 [details] Bug 1330829 - Login Manager recipe support for ignoring specific password fields. https://reviewboard.mozilla.org/r/104368/#review105134 > nit: The combination of "," and "+" concatenating looks a bit random here, can't you replace the single "+" with a ","? The problem is that I didn't want a space before the ")" and spaces get added between arguments. > nit: Since you've added trailing commas in most other parts of your patch, I assume you forgot to add one here :) Yep, I like trailing commas.
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/mozilla-inbound/rev/fc4d5c2cade3 Login Manager recipe support for ignoring specific password fields. r=johannh
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fc4d5c2cade3
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•