Bug 89209 - REGRESSION(r112113): absolutely positioned INPUT boxes with a table cell containing block have a 0px height
Summary: REGRESSION(r112113): absolutely positioned INPUT boxes with a table cell cont...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.7
: P2 Normal
Assignee: Julien Chaffraix
URL: http://jsfiddle.net/mqJhw/7/
Keywords: HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2012-06-15 07:15 PDT by fred
Modified: 2012-11-13 00:07 PST (History)
11 users (show)

See Also:


Attachments
Reduction (440 bytes, text/html)
2012-07-10 14:28 PDT, Julien Chaffraix
no flags Details
Proposed fix, reset overriden height before laying out. (5.46 KB, patch)
2012-07-10 15:55 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description fred 2012-06-15 07:15:48 PDT
Create a positioned container, e.g. DIV of say 100 x 100px and place an INPUT inside it. Style the input with { position: absolute; width: auto; left:0; right:0; height: auto; top: 0; bottom:0; border: none; background: none }  Now observe on a Mac when the page loads it does not show the Value of the INPUT tag. When you hover over it you can see the blue outline of where the INPUT element should be, but you cannot click inside it to edit the text. But this does work fine on the same version of Chrome for Windows. Adding another style rule: { min-height: 20px } will fix it, but this should not be necessary.

In the http://jsfiddle.net/mqJhw/7/ version the min-height workaround is disabled, while in http://jsfiddle.net/mqJhw/6/ it is enabled.
Comment 1 fred 2012-06-15 07:23:50 PDT
Forgot to mention, that the rendering error exists in both Chrome Canary 21.0.1175.0 and Webkit nightly build r120398, but not in Safari 5.1.7!
Comment 2 Kent Tamura 2012-06-18 19:04:53 PDT
Regression range: http://trac.webkit.org/log/trunk/?rev=112129&stop_rev=112101&verbose=on
Comment 3 Kent Tamura 2012-06-28 21:46:00 PDT
I confirmed this was a regression by http://trac.webkit.org/changeset/112113 .
Julien?
Comment 4 Julien Chaffraix 2012-07-09 19:05:32 PDT
(In reply to comment #3)
> I confirmed this was a regression by http://trac.webkit.org/changeset/112113 .
> Julien?

It looks like the issue was already present but uncovered by the changeset. The core of the issue is that we override the style's height in RenderTextControlSingleLine::layout. In this case, because of the input's containing block is a cell, it hasn't been laid out yet and we improperly constrain the <input>'s inner blocks to have a 0 height. The second layout unfortunately re-uses the previous (wrong) value and over-constrain.

I fear that to fix the regression, we need to remove the height-override hacks (which would be a good change but non-trivial). A system akin to cell's intrinsic padding should give us what we want.
Comment 5 Julien Chaffraix 2012-07-10 14:28:30 PDT
Created attachment 151526 [details]
Reduction
Comment 6 Julien Chaffraix 2012-07-10 15:55:52 PDT
Created attachment 151544 [details]
Proposed fix, reset overriden height before laying out.
Comment 7 WebKit Review Bot 2012-07-10 20:11:49 PDT
Comment on attachment 151544 [details]
Proposed fix, reset overriden height before laying out.

Clearing flags on attachment: 151544

Committed r122290: <http://trac.webkit.org/changeset/122290>
Comment 8 WebKit Review Bot 2012-07-10 20:11:58 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Chris Dumez 2012-07-11 00:10:58 PDT
On EFL and GTK ports, the test is failing because we cannot see the date in the second <input>.
Comment 10 Chris Dumez 2012-07-11 00:13:37 PDT
(In reply to comment #9)
> On EFL and GTK ports, the test is failing because we cannot see the date in the second <input>.

Same for Qt port.
Comment 11 Zan Dobersek 2012-07-11 00:33:22 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > On EFL and GTK ports, the test is failing because we cannot see the date in the second <input>.
> 
> Same for Qt port.

The second input is of type 'date'[1]. I believe for the test to properly function, the ENABLE_INPUT_TYPE_DATE feature should be enabled. Most ports don't enable it, though, so that's probably why the test fails:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=fast%2Fforms%2Finput-in-table-cell-no-value.html

[1] - http://trac.webkit.org/browser/trunk/LayoutTests/fast/forms/input-in-table-cell-no-value.html?rev=122290#L29
Comment 12 Kent Tamura 2012-07-11 00:42:24 PDT
(In reply to comment #11)
> The second input is of type 'date'[1]. I believe for the test to properly function, the ENABLE_INPUT_TYPE_DATE feature should be enabled. Most ports don't enable it, though, so that's probably why the test fails:
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=fast%2Fforms%2Finput-in-table-cell-no-value.html

I don't this so. If a port has no INPUT_TYPE_DATE, <input type=date> should work as <input type=text>, and the test HTML and the reference HTML produce the identical image.

Probably repainting issue?
Comment 13 Zan Dobersek 2012-07-11 00:47:30 PDT
(In reply to comment #12)
> 
> Probably repainting issue?

I'm not sure that's the issue. When running the test and expected HTML manually the difference still occurs. Expected HTML renders the date in text but the test HTML doesn't.
Comment 14 Kent Tamura 2012-07-11 00:53:40 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > 
> > Probably repainting issue?
> 
> I'm not sure that's the issue. When running the test and expected HTML manually the difference still occurs. Expected HTML renders the date in text but the test HTML doesn't.

I see.
type=date and type=text have some differences. Probably the patch fixed type=number and type=date case, but didn't fix type=text case.
The patch itself isn't harmful.  I think we had better skip the test on ports without INPUT_TYPE_DATE, and fix the type=text case later.
Comment 15 Julien Chaffraix 2012-07-11 09:31:42 PDT
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > 
> > > Probably repainting issue?
> > 
> > I'm not sure that's the issue. When running the test and expected HTML manually the difference still occurs. Expected HTML renders the date in text but the test HTML doesn't.
> 
> I see.
> type=date and type=text have some differences. Probably the patch fixed type=number and type=date case, but didn't fix type=text case.
> The patch itself isn't harmful.  I think we had better skip the test on ports without INPUT_TYPE_DATE, and fix the type=text case later.

Looks like the test was disabled on non chromium platforms, sorry for the breakage. I have filed bug 90987 to track the failure. I hope to look at it after a couple of high-priority bugs on my plate.
Comment 16 mitz 2012-11-13 00:07:22 PST
<rdar://problem/12685932>