[Webkit-unassigned] [Bug 104600] [Chromium] Always enable autocomplete for password fields

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=104600





--- Comment #32 from Garrett Casto <gcasto at chromium.org>  2012-12-28 13:24:32 PST ---
(In reply to comment #31)
> (In reply to comment #29)
> > Maybe it would be helpful for me to see how you will use this code from the Chromium side, but just looking at how WebPasswordFormData gets used, it looks like content::CreatePasswordForm is the only place where WebPasswordFormData gets instantiated.
> > 
> > See content::CreatePasswordForm here:
> > http://code.google.com/searchframe#OAMlx_jo-ck/src/content/renderer/password_form_conversion_utils.cc&exact_package=chromium&q=CreatePasswordForm&type=cs%22&l=42
> > 
> > It seems like you could pass a boolean parameter to that function, and then forward it to the WebPasswordFormData constructor.
> > 
> > In other words, it appears that you only need to ignore autocomplete=off while constructing the WebPasswordFormData, so it seems simpler to just pass a boolean flag to it rather than messing with WebSettings.
> 
> I guess my question is: can we easily distinguish whether it's called by chrome or called by other content embedders when we call CreatePasswordForm()? I saw it is called 12 times in the code base, some under chrome/ while some under content/. Are you suggesting if it is called under chrome/ we pass "true" (means ignore) to the parameter and if it is called under content/ we pass "false"?. If so, I think using WebSettings would be easier since we don't need to change the parameter every place we call it. Thoughts?

I only see 3 files that have calls to CreatePasswordForm() (https://cs.corp.google.com/#search/&q=CreatePasswordForm%5C(%20package:%5Echrome$&type=cs). Only one is in content (content/renderer/render_view_impl.cc) but this needs to ignore autocomplete=off for Chrome as it's how the password_manager is notified. So it doesn't seem like changing the constructor helps here.

Also, are all of these other embedders located in the chrome repository? Because if so, I don't think that they are consuming this data (see https://cs.corp.google.com/#search/&q=%5C.password_form%20package:%5Echrome$&type=cs). Given that, and the fact that you already have to check to see if the <form> element has autocomplete=off set even without this change, would it make sense just to add a comment in WebPasswordFormData/PasswordForm explaining that the caller needs to deal with autocomplete?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.