While working on form fill I noticed that the login manager does not always fill in the username field. The reason is because it only considers input types of type="text" right now. But it should accept other input types too. This fix (attaching shortly) is similar to existing code already used on desktop firefox here: http://dxr.mozilla.org/mozilla-central/toolkit/components/passwordmgr/nsLoginManager.js.html#l668

Comment on attachment 708791 [details] [diff] [review] Patch v1. Review of attachment 708791 [details] [diff] [review]: ----------------------------------------------------------------- looks pretty solid, (the f+ not r+ is more a reflection on me than you) and I think the whitelist approach is the way to go, however: - There is no test with this. I feel like this is something we could have caught with an automated test easily. I would be fine with a) a test b) a follow up bug to write a test c) a reason why we are unable to write a test - Optionally, if you have it, it would be nice to list some sites other than gmail, STRs, or anything else that might help qa. - I don't think I know this code well enough to qualify as a reviewer of it. So, I'm going to find you someone else to do a driveby, and if it looks good to them, then you can either take their r+ or morph my f+ into an r+. question: the msg lists timA as the reviewer. Did you mean to have him review the patch as well?

Thanks Ally, I'll post a followup for a test. I was originally going to put this login manager patch for review by TimA but I couldn't get his name to come up in autocomplete so I picked you!!! Lucky! :P For step to reproduce, you can just have an <input type="email"> for a form with a password after it. In that case the username will not be autofilled because it can't find the username field because it is not <input type="text">

Comment on attachment 708791 [details] [diff] [review] Patch v1. Review of attachment 708791 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/metro/base/content/LoginManagerChild.js @@ +326,5 @@ > + if (fieldType == "text" || > + fieldType == "email" || > + fieldType == "url" || > + fieldType == "tel" || > + fieldType == "number") { I think that ((element instanceof HTMLInputElement && element.mozIsTextField(true)) is a better way to write this, unless there's some reason I'm not seeing that desktop Firefox did it the other way. http://mxr.mozilla.org/mozilla-central/ident?i=mozIsTextField Seconding ally's request for a test.

Test bug is here: bug 837295. I'll work on those tests Monday or possibly over the weekend.

Comment on attachment 708791 [details] [diff] [review] Patch v1. Okay, I see the mozIsTextField option was debated on desktop, and more-or-less concluded with bug 600551 comment 15. Let's just copy desktop.

\o/ and thank you for the prompt test bug :)