Bug 84009 – Introduce an internal feature for a fixed placeholder

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

[Kent Tamura]

Introduce an internal feature for a fixed placeholder

[Kent Tamura]

Created attachment 137282 [details]
Patch

[Kentaro Hara]

Comment on attachment 137282 [details]
Patch

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

> Source/WebCore/html/InputType.h:243
> +    // If supportsPlaceholder() && defaultPlaceholder().isEmpty(), it means a
> +    // type supports the 'placeholder' attribute.
> +    // If supportsPlaceholder() && !defaultPlaceholder().isEmpty(), it means a
> +    // type doesn't support the 'placeholder' attribute, but shows
> +    // defaultPlaceholder() string as a placeholder.

Nit: It might be better to put this comment above supportsPlaceholder().

> Source/WebCore/html/TextFieldInputType.cpp:397
> +    if (placeholderText.isEmpty())

Shouldn't this be 'if (!placeholderText.isEmpty())'?

[Kent Tamura]

Comment on attachment 137282 [details]
Patch

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

>> Source/WebCore/html/InputType.h:243
>> +    // defaultPlaceholder() string as a placeholder.
> 
> Nit: It might be better to put this comment above supportsPlaceholder().

It's reasonable.  I'll do.

>> Source/WebCore/html/TextFieldInputType.cpp:397
>> +    if (placeholderText.isEmpty())
> 
> Shouldn't this be 'if (!placeholderText.isEmpty())'?

No.  If defaultPlaceholder() is empty, we'll use the 'placeholder' attribute value.

[Kent Tamura]

Created attachment 137302 [details]
Patch 2

Fix a comment

[Kent Tamura]

Created attachment 137303 [details]
Patch 3

[Kentaro Hara]

Comment on attachment 137282 [details]
Patch

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

>>> Source/WebCore/html/TextFieldInputType.cpp:397
>>> +    if (placeholderText.isEmpty())
>> 
>> Shouldn't this be 'if (!placeholderText.isEmpty())'?
> 
> No.  If defaultPlaceholder() is empty, we'll use the 'placeholder' attribute value.

Ah, got it. I'd been confused.

> Source/WebCore/html/TextFieldInputType.cpp:398
> +        placeholderText = element()->strippedPlaceholder();

Nit: You can move this statement inside the below if statement.

[Kentaro Hara]

(In reply to comment #6)
> > Source/WebCore/html/TextFieldInputType.cpp:398
> > +        placeholderText = element()->strippedPlaceholder();
> 
> Nit: You can move this statement inside the below if statement.

Sorry, ignore this comment. You code is right.

[Kent Tamura]

Created attachment 137486 [details]
Patch 4

Add InputType::usesFixedPlaceholder()

[WebKit Review Bot]

Comment on attachment 137486 [details]
Patch 4

Clearing flags on attachment: 137486

Committed r114360: <http://trac.webkit.org/changeset/114360>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.