[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 #23 from Yue Zhang <zysxqn at chromium.org>  2012-12-20 16:28:30 PST ---
(In reply to comment #21)
> (In reply to comment #20)
> > (In reply to comment #19)
> > > (In reply to comment #16)
> > > > (In reply to comment #15)
> > > > > (In reply to comment #14)
> > > > > > (In reply to comment #12)
> > > > > > > Isn't that a chrome/ feature (as opposed to content/)? Shouldn't we in that case make this somehow configurable?
> > > > > > > 
> > > > > > > Also, what about tests?
> > > > > > 
> > > > > > Yeah it's a chrome feature. 
> > > > > 
> > > > > Ok, then it should be configurable - otherwise other content embedders might end up just ignoring this field
> > > > 
> > > >   I see WebPasswordFormUtils.cpp is under chromium/ directory so I thought it's only used by chromium code. No? If not, how to make it configurable? command line flags? I'm really new to WebKit so any more detailed suggestion is welcomed :)
> > > 
> > > 
> > > The WebKit chromium/ port is used for a number of non-desktop chrome projects such as chrome on android, content_shell, etc..
> > > 
> > > You could add a bool to WebSettings.h that controls this behavior and defaults to honoring the autocomplete attribute.
> > > 
> > > In WebPasswordFormElement, you can access the settings with document()->frame()->view()->settings()
> > > 
> > 
> > Thanks for your detailed explanation. Just one question, where could we change it to the non-default value for our chrome feature? I did a code search and it looks like we should use WebView::GetWebSettings() to change its value but it is not called anywhere currently in the code base? Thanks!
> 
> You need to add the new setting in webkit/glue/webpreferences.*, and add the new field to content/public/common/content_param_traits_macros.h
> 
> then you can override your new setting in chrome/browser/chrome_content_browser_client.cc in the OverrideWebkitPrefs method

Thanks! I have made it configurable through WebSettings. Please have another look. The relevant chrome change (11635061) has also been uploaded and assigned to you as the reviewer. Please also have a look. Thanks!
> 
> > 
> > > 
> > > > > 
> > > > > > 
> > > > > > Regarding test, I'm not super familiar with Webiit code. I think this is a pretty trivial change. Shall we reflect it somewhere in the Webkit unittest?
> > > > > 
> > > > > sounds good. there are unit tests in http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/tests/
> > > > 
> > > > Looks that there is no existing tests regarding WebPasswordFormUtils class. Is it really worth a new test file given that it is a trivial two-line change?
> > > 
> > > It worries me that ignoring the autocomplete attribute doesn't break any test (also, if it's worthwhile to change it, it should be worthwhile to test it)
> > > 
> > > Alternatively, you can try to create a layout test, whatever you prefer

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