Bug 22348 – -webkit-autofill pseudo style not always respected

• Component: CSS
• Status: RESOLVED FIXED

[Greg Bolsinga]

This happens in Safari 3.2 and TOT.

Open the attached form. Go to Edit->AutoFill Form. Only the first name field is en-yellowed. I think all of them should be, as each is getting the -webkit-autofill pseudo style set via HTMLInputElement::setAutofilled(true).

If I hack up this property to always be true, the attached page will load with all en-yellowed fields. I verified in the debugger that autofill is not being set to false.

[Greg Bolsinga]

Created attachment 25254 [details]
forms that will not en-yellow after autofill command

[Greg Bolsinga]

<rdar://problem/6383673>

[Antti Koivisto]

<rdar://problem/6383673>

[David Kilzer (:ddkilzer)]

<rdar://problem/6383673>

[Mark Rowe (bdash)]

<rdar://problem/6383673>

[Simon Fraser (smfr)]

I have a fix.

[Simon Fraser (smfr)]

Created attachment 25286 [details]
Patch, changelog

It's hard to make a testcase since there's no way to set the -webkit-autofill pseudostyle from JS (that I know of), and making autofill happen is a Safari-level thing.

[Darin Adler]

Comment on attachment 25286 [details]
Patch, changelog

>      virtual bool isControl() const { return false; } // Eventually the notion of what is a control will be extensible.
>      virtual bool isEnabled() const { return true; }
>      virtual bool isChecked() const { return false; }
> +    virtual bool isAutofilled() const { return false; }
>      virtual bool isIndeterminate() const { return false; }
>      virtual bool isReadOnlyControl() const { return false; }
>      virtual bool isTextControl() const { return false; }

Not sure why you chose this point in the list to add it. Not alphabetical. Between checked and indeterminate seems a strange choice, since those two are closely related.

>                  if (e && e->hasTagName(inputTag))
> -                    return static_cast<HTMLInputElement*>(e)->autofilled();
> +                    return static_cast<HTMLInputElement*>(e)->isAutofilled();

If we're going to make isAutofilled virtual, then we should remove the hasTagName check here. It's just there as a type check so we can call the non-virtual function. We could also have put that check inside the code in canShareStyleWithElement.

> +void HTMLInputElement::setAutofilled(bool b)
> +{
> +    if (b != m_autofilled) {
> +        m_autofilled = b;
> +        setChanged();
> +    }
> +}

We normally use early return for something like this rather than nesting both lines inside an if statement.

r=me as is, but you could consider refining too

[Simon Fraser (smfr)]

Refined and committed:

Committed r38602
	M	WebCore/dom/Node.h
	M	WebCore/ChangeLog
	M	WebCore/html/HTMLInputElement.cpp
	M	WebCore/html/HTMLInputElement.h
	M	WebCore/css/CSSStyleSelector.cpp
r38602 = 5c9b448d855fb099fc4e0ab58c0327463a6c5ffd (trunk)