We want to be able to create a formLike object from a username field. To do that that we need to change the FormLikeFactory.createFromPasswordField method to also accept input fields of type text.

Bug 1192081 - Changed createFromPasswordField to also create formlikes from text fields. r=MattN

Comment on attachment 8646066 [details] MozReview Request: Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN Bug 1192081 - Changed createFromPasswordField to also create formlikes from text fields. r=MattN

Comment on attachment 8646066 [details] MozReview Request: Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN https://reviewboard.mozilla.org/r/15607/#review13983 ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:1237 (Diff revision 2) > - aPasswordField.type != "password" || > + (aInputField.type != "password" && aInputField.type != "text") || Please use the existing username field rules which support more than type=text like I mentioned in our 1:1. See `_isUsernameFieldType` ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:1235 (Diff revision 2) > - createFromPasswordField(aPasswordField) { > + createFromInputField(aInputField) { "input" and "field" are somewhat redundant. How about createFromField with aField (which can include other fields like <select> and <textarea> that I think we will want to support eventually)? ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:1223 (Diff revision 2) > * Create a FormLike object from an <input type=password>. This is stale ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:615 (Diff revision 2) > + * Note that even than we can create a formLike from a text field, s/even than/even though/ ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:617 (Diff revision 2) > + * formLike have a password field. s/have/has/ ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:615 (Diff revision 2) > + * Note that even than we can create a formLike from a text field, > + * this method will only return a non-null usernameField if the > + * formLike have a password field. NitL Capitalize FormLike when referring to the data type. ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:1233 (Diff revision 2) > - * @throws Error if aPasswordField isn't a password input in a document > + * @throws Error if aInputField isn't a password input in a document Stale comment ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:1231 (Diff revision 2) > - * @param {HTMLInputElement} aPasswordField - a password field in a document > + * @param {HTMLInputElement} aInputField - a password field in a document Stale comment. This should be a username or password input? ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:1239 (Diff revision 2) > - throw new Error("createFromPasswordField requires a password field in a document"); > + throw new Error("createFromInputField requires an input field in a document"); username or password field ::: toolkit/components/passwordmgr/test/test_formless_submit.html:57 (Diff revision 2) > document: `<input value="user1"> > <input type=password value="pass1">`, > + inputIndexForFormLike: 0, > + hostname: DEFAULT_ORIGIN, > + formSubmitURL: DEFAULT_ORIGIN, > + usernameFieldValue: "user1", > + newPasswordFieldValue: "pass1", > + oldPasswordFieldValue: null, > + }, > + { > + document: `<input value="user1"> > + <input type=password value="pass1">`, > inputIndexForFormLike: 1, > hostname: DEFAULT_ORIGIN, > formSubmitURL: DEFAULT_ORIGIN, > usernameFieldValue: "user1", > newPasswordFieldValue: "pass1", > oldPasswordFieldValue: null, > }, These two cases looks the same. What am I missing? ::: toolkit/components/passwordmgr/test/unit/test_getFormFields.js:21 (Diff revision 2) > + description: "1 text field outside of a <form>", "…without a password field" (be explicit as I can see someone expecting this to work) ::: toolkit/components/passwordmgr/test/unit/test_getPasswordFields.js:19 (Diff revision 2) > - returnedFieldIDsByFormLike: [], > + // Only the IDs of password fields should be at this array Nit: s/at/in/ ::: toolkit/components/passwordmgr/LoginManagerContent.jsm:1230 (Diff revision 2) > * Might as well update the commit message too: * update method name * s/text/username/ * s/formlikes/FormLikes/

Comment on attachment 8646066 [details] MozReview Request: Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN

https://reviewboard.mozilla.org/r/15607/#review13983 Thank you for your review, Matt! I moved `_isUsernameFieldType` from the `LoginManagerContent` object to the `LoginUtils` object, as it's a usefull function outside that object too. > These two cases looks the same. What am I missing? In the first case we use the username input to create the FormLike. In the second case we use the password input to create the FormLike. They are almost the same, but this difference allow us to know if createFromField is doing the right thing for usernames too.

Comment on attachment 8646066 [details] MozReview Request: Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN

New revision with the `_isUsernameFieldType` method moved to LoginHelper, so we can also use it elsewhere. I will need to use this on `nsContextMenu.js` for bug 1188719.

Comment on attachment 8646066 [details] MozReview Request: Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN https://reviewboard.mozilla.org/r/15607/#review14105 Ship It!

Comment on attachment 8646066 [details] MozReview Request: Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN Approval Request Comment [Feature/regressing bug #]: Dependency of bug 1188719 [User impact if declined]: Not able to fill logins from a non-password field e.g. username field. [Describe test coverage new/current, TreeHerder]: Updated unit tests [Risks and why]: Low risk change to make non-password fields work just like password fields already do. [String/UUID change made/needed]: None

Comment on attachment 8646066 [details] MozReview Request: Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN Seems like it didn't cause any regression, taking it for the other patch. Should be in 42 beta 5

This cause conflicts while uplifting to beta: grafting 257353:87cace3f3eac "Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN" merging toolkit/components/passwordmgr/LoginHelper.jsm warning: conflicts during merge. merging toolkit/components/passwordmgr/LoginHelper.jsm incomplete! (edit conflicts, then use 'hg resolve --mark') merging toolkit/components/passwordmgr/LoginManagerContent.jsm abort: unresolved conflicts, can't continue could you take a look, thanks!

matt: since this is planned to be in beta, could you take a look

(In reply to Carsten Book [:Tomcat] from comment #14) > matt: since this is planned to be in beta, could you take a look I was already trying to land this yesterday but the tree was closed.