Bug 61768 – REGRESSION (r87067): Text overflows from short height text field

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

[Yuzo Fujishima]

Created attachment 95400 [details]
Reduction

Text typed into a short height text field overflows from the field to below.

Reproduction steps:
1. Open the attached reduction with Safari (WebKit 87713).
2. Observe the text field.

Firefox 4.0.1 and Opera 11.11 behave correctly -- they render text only inside the text field.

[Yuzo Fujishima]

Created attachment 95401 [details]
Screenshot

[Kent Tamura]

I think we need to add overflow-y:hidden or something to the UA stylesheet for <input>.

[Yuzo Fujishima]

Created attachment 95424 [details]
Patch

This is a fairly recent regression (the bug doesn’t occur in r86179). I expect the lightweight control clip mechanism to do this automatically. I think it’s important to understand how the regression occurred before attempting this fix.

Specifically, it seems likely that this was caused by r87067.

<rdar://problem/9527476>

Comment on attachment 95424 [details]
Patch

This should be handled by hasControlClip() / controlClipRect().

[Kent Tamura]

(In reply to comment #4)
> I think it’s important to understand how the regression occurred before attempting this fix.

At the beginning of RenderTextControlSingleLine::layout(), we change the inner text height to 1 by innerTextRenderer->style()->setHeight(Length(desiredheight, Fixed)) in this case.  I confirmed this was called on ToT, but it was not effective?

I would investigate whether hasControlClip() and controlClipRect() are still consulted and what values they return. This is something that happens at paint time, not layout time.

[Kent Tamura]

Well, I found the root cause.

As I wrote in Comment #8, RenderTextControlSingleLine::layout() adjusts the height of the inner text block. However TextControlInnerTextElement::styleForRenderer() resets the RenderStyle since r87067. It shows the problem in a case that:
 1. RenderTextControlSingleLine::layout() is done.
    RenderStyle for the inner text has height=1.
 2. HTMLInputElement::value is updated.
    So, TextControlInnerTextElement::styleForRenderer() resets the RenderStyle.  It has no height.
 3. The inner text is laid out and it has a natural height such as 13px,
   but RenderTextControlSingleLine::layout() is not called (is it normal?)



> I would investigate whether hasControlClip() and controlClipRect() are still consulted and what values they return.

We have never used the control clip to clip the inner text of <input> AFAIK.

[Kent Tamura]

(In reply to comment #10)
> Well, I found the root cause.
> 
> As I wrote in Comment #8, RenderTextControlSingleLine::layout() adjusts the height of the inner text block. However TextControlInnerTextElement::styleForRenderer() resets the RenderStyle since r87067. It shows the problem in a case that:
>  1. RenderTextControlSingleLine::layout() is done.
>     RenderStyle for the inner text has height=1.
>  2. HTMLInputElement::value is updated.
>     So, TextControlInnerTextElement::styleForRenderer() resets the RenderStyle.  It has no height.
>  3. The inner text is laid out and it has a natural height such as 13px,
>    but RenderTextControlSingleLine::layout() is not called (is it normal?)

Correction: RenderTextControlSingleLine::layout() was called. But the RenderBlock for the inner text still has an old small height.  So setHeight() is not called.

[Kent Tamura]

Created attachment 95547 [details]
Patch 2

[Kent Tamura]

I found the patch causes another problem.

[Kent Tamura]

Created attachment 99607 [details]
Patch

[WebKit Review Bot]

Comment on attachment 99607 [details]
Patch

Attachment 99607 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8979717

New failing tests:
fast/forms/textfield-overflow-by-value-update.html

[WebKit Review Bot]

Created attachment 99608 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Kent Tamura]

Created attachment 99609 [details]
Patch 4

The test fails on Chromium expectedly

[Dimitri Glazkov (Google)]

Comment on attachment 99609 [details]
Patch 4

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

> LayoutTests/fast/forms/textfield-overflow-by-value-update.html:15
> +     So, the test result should be just white. -->

That's pretty clever :)

[Kent Tamura]

Comment on attachment 99609 [details]
Patch 4

Thank you for r+!

[WebKit Review Bot]

Comment on attachment 99609 [details]
Patch 4

Clearing flags on attachment: 99609

Committed r90379: <http://trac.webkit.org/changeset/90379>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.