Bug 138682 – REGRESSION (r172826) Password field placeholder text is missing

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Jon Honeycutt]

Pasword field placeholder text is missing if the placeholder attribute precedes the type attribute.

To repro, navigate to data:text/html,<input placeholder="Password" type="password">


<rdar://problem/18574213>

[Jon Honeycutt]

Created attachment 241463 [details]
Patch

[Benjamin Poulain]

Comment on attachment 241463 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=241463&action=review

Thanks a lot for fixing this!

> LayoutTests/ChangeLog:12
> +        * fast/forms/placeholder-attribute-ordering-expected.html: Added.
> +        * fast/forms/placeholder-attribute-ordering.html: Added.

Can you please add a test were attributes are added dynamically from JS?

Dynamic style updates tend to have their own set of bugs.

> LayoutTests/fast/forms/placeholder-attribute-ordering.html:5
> +        ::-webkit-input-placeholder { color: green; }

Do you really need to color the placeholder for testing?

Using the pseudo element ::-webkit-input-placeholder is a bit annoying because its scope is not yet well defined.

[Jon Honeycutt]

(In reply to comment #2)
> Comment on attachment 241463 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241463&action=review
> 
> Thanks a lot for fixing this!
> 
> > LayoutTests/ChangeLog:12
> > +        * fast/forms/placeholder-attribute-ordering-expected.html: Added.
> > +        * fast/forms/placeholder-attribute-ordering.html: Added.
> 
> Can you please add a test were attributes are added dynamically from JS?

Yes, I'll add some tests for this.

> 
> Dynamic style updates tend to have their own set of bugs.
> 
> > LayoutTests/fast/forms/placeholder-attribute-ordering.html:5
> > +        ::-webkit-input-placeholder { color: green; }
> 
> Do you really need to color the placeholder for testing?
> 
> Using the pseudo element ::-webkit-input-placeholder is a bit annoying
> because its scope is not yet well defined.

I wanted to use the 'placeholder' attribute for the test and the 'value' attribute to display the same text for the reference file, so I styled them to look the same. I'll change them to both use the 'placeholder' attribute, though.

Thanks for the review!

[Jon Honeycutt]

Committed r176082: <http://trac.webkit.org/changeset/176082>