Speed up HTMLInputElement::isEmptyValue()

• Component: DOM
• Status: RESOLVED FIXED

[Chris Dumez]

HTMLInputElement::isEmptyValue() currently calls HTMLTextFormControlElement::innerTextValue() which causes a full subtree traversal to construct a string representation of that subtree using a StringBuilder. In the case of HTMLInputElement::isEmptyValue(), we shouldn't have to do all this: We don't need to construct a String and we can return false as soon as we find a non-empty descendant.

[Chris Dumez]

Created attachment 241212 [details]
Patch

[Geoffrey Garen]

Comment on attachment 241212 [details]
Patch

r=me

[Chris Dumez]

Comment on attachment 241212 [details]
Patch

Clearing flags on attachment: 241212

Committed r175778: <http://trac.webkit.org/changeset/175778>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 241212 [details]
Patch

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

> Source/WebCore/html/HTMLInputElement.cpp:1684
> +    if (!isTextField())
> +        return true;

I suggest putting this code into a virtual InputType::isEmptyValue instead of using the virtual InputType::isTextField to do a boolean check and putting the algorithm here.

> Source/WebCore/html/HTMLInputElement.cpp:1691
> +        if (text->length())
> +            return false;

There’s no whitespace collapsing rule we have to consider here?

[Chris Dumez]

Comment on attachment 241212 [details]
Patch

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

>> Source/WebCore/html/HTMLInputElement.cpp:1684
>> +        return true;
> 
> I suggest putting this code into a virtual InputType::isEmptyValue instead of using the virtual InputType::isTextField to do a boolean check and putting the algorithm here.

Sounds good.

>> Source/WebCore/html/HTMLInputElement.cpp:1691
>> +            return false;
> 
> There’s no whitespace collapsing rule we have to consider here?

I did not change behavior. It was previously calling innerTextValue().isEmpty() which returns true only of the actual empty string. This function is used to determine if the placeholder should be visible or not. I would expect the placeholder to not be visible if the input contains white spaces (and as I said, this was WebKit's behavior already).