Input value/placeholder is not redrawn when the input height grows

• Component: Forms
• Status: RESOLVED FIXED

[Kent Tamura]

https://code.google.com/p/chromium/issues/detail?id=126395

<input value=foo style="height:0.5em;">
<input placeholder=foo style="height:0.5em;">
<script>
document.getElementsByTagName('input')[0].style.height = 'initial';
document.getElementsByTagName('input')[1].style.height = 'initial';
</script>

The value/placeholder keeps the initial height after setting input height.

[Kent Tamura]

Created attachment 196278 [details]
Patch

[Ojan Vafai]

Comment on attachment 196278 [details]
Patch

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

> Source/WebCore/rendering/RenderTextControlSingleLine.cpp:256
> +    if (RenderObject* innerTextRenderer = innerTextElement()->renderer())
> +        innerTextRenderer->setNeedsLayout(true, MarkOnlyThis);

I think we only need this if stye StyleDifference is a Layout difference.

[Kent Tamura]

Created attachment 196282 [details]
Patch 2

StyleDifferenceLayout

[Kent Tamura]

Comment on attachment 196278 [details]
Patch

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

>> Source/WebCore/rendering/RenderTextControlSingleLine.cpp:256
>> +        innerTextRenderer->setNeedsLayout(true, MarkOnlyThis);
> 
> I think we only need this if stye StyleDifference is a Layout difference.

That's right!

[Ojan Vafai]

Comment on attachment 196282 [details]
Patch 2

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

> Source/WebCore/rendering/RenderTextControlSingleLine.cpp:257
> +        innerTextRenderer->setNeedsLayout(true, MarkOnlyThis);

You shouldn't MarkOnlyThis. I'm trying to get rid of all of our uses of MarkOnlyThis. There are a couple cases where we need it right now to avoid hitting asserts, but setNeedsLayout in styleDidChange/styleWillChange it should be safe to leave out the MarkOnlyThis.

[Kent Tamura]

Comment on attachment 196282 [details]
Patch 2

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

>> Source/WebCore/rendering/RenderTextControlSingleLine.cpp:257
>> +        innerTextRenderer->setNeedsLayout(true, MarkOnlyThis);
> 
> You shouldn't MarkOnlyThis. I'm trying to get rid of all of our uses of MarkOnlyThis. There are a couple cases where we need it right now to avoid hitting asserts, but setNeedsLayout in styleDidChange/styleWillChange it should be safe to leave out the MarkOnlyThis.

Yeah, I agree it.  This is a hack.
I think we can remove many hacks in RenderTextControlSingleLine by using flexible box with align-items:center.

[WebKit Review Bot]

Comment on attachment 196282 [details]
Patch 2

Clearing flags on attachment: 196282

Committed r147602: <http://trac.webkit.org/changeset/147602>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.