[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 #27 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2012-12-20 17:05:12 PST ---
(In reply to comment #26)
> (In reply to comment #25)
> > (From update of attachment 180437 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=180437&action=review
> > 
> > > Source/WebCore/page/Settings.cpp:422
> > > +void Settings::setIgnoreWebkitAutocompleteOff(bool ignoreWebkitAutocompleteOff)
> > 
> > Why is the substring "Webkit" in this method name?
> 
> Because we don't completely ignore autocomplete=off, just don't check/handle this in webkit (will handle this in chrome code). So we choose the name like this.

But, you are checking this in WebKit.  WebPasswordFormUtils.cpp is part of WebKit.

That said, I still don't understand how "Webkit" in the name helps convey your intended meaning.


> > 
> > > Source/WebKit/chromium/src/WebPasswordFormUtils.cpp:88
> > > +                || inputElement->shouldAutocomplete())) {
> > 
> > Perhaps the implementation of shouldAutocomplete should inspect the ignoreAutocompleteOff setting?
> > That way, every caller of shouldAutocomplete will see the same value, and we will not have to
> > repeat the settings test at each callsite of shouldAutocomplete.
> 
> The whole point of not checking autocomplete in webkit is that we don't have enough information in webkit code (we ignore autocomplete setting for chrome generated passwords, but respect it for other normal passwords). So for the same reason, I don't see how we can handle autocomplete setting in the implementation of shouldAutocomplete? maybe I miss anything?

Hmm, I thought all password filling when through this code path.  That said, it seems like shouldAutocomplete is used to control whether or not other form state is saved when navigating away from a page.  It makes sense to still honor the setting in those cases.  I understand now why shouldAutocomplete would not check this preference.

Next, I wonder why you need to store this setting on WebCore::Settings if the setting is never consulted by WebCore.  It seems like we could instead just pass a boolean ignoreAutocompleteOff parameter to WebPasswordFormData's constructor, right?

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