[webkit-reviews] review denied: [Bug 104997] REGRESSION(r90089): Text in <input> overflows if padding is updated after layout : [Attachment 179480] Patch 2

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Kent Tamura <tkent at chromium.org>'s
request for review:
Bug 104997: REGRESSION(r90089): Text in <input> overflows if padding is updated
after layout
https://bugs.webkit.org/show_bug.cgi?id=104997

Attachment 179480: Patch 2
https://bugs.webkit.org/attachment.cgi?id=179480&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=179480&action=review


I did some digging and found out that the bug is more general.	It happens to
all box-sizing: border-box cases.  Here's an example:
http://jsfiddle.net/TCFCf/

I think the fix is simple.  In RenderBlock::updateLogicalWidthAndColumnWidth,
we compare the logicalWidth before and after.  I think we should be comparing
the logicalContentWidth instead.  Hmm, we'll have to test to see if that causes
other unintended side effects.

> LayoutTests/fast/forms/text/text-padding-dynamic-change.html:11
> +if (window.testRunner)
> +    testRunner.waitUntilDone();
> +window.onload = function() {
> +    setTimeout(function() {

You shouldn't need to use waitUntilDone or setTimeout. In the <script>, force a
layout using document.body.offsetHeight, then change the padding.  You don't
even need to wait for onload.

You could also make this a check-layout.js test (it has slightly nicer output
with PASS/FAIL), but a ref-test is also OK.