[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 #21 from jochen at chromium.org  2012-12-19 14:07:34 PST ---
(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

> 
> > 
> > > > 
> > > > > 
> > > > > 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.