Bug 8072

Summary: REGRESSION: text fields at connect.apple.com spill out of the containing box
Product: WebKit Reporter: Henk <henk.kampman>
Component: Layout and RenderingAssignee: Adele Peterson <adele>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, darin, henk.kampman, hyatt
Priority: P1 Keywords: Regression
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://connect.apple.com/
Attachments:
Description Flags
reduction, showing this is some sort of min/max width issue
none
initial patch hyatt: review+

Description Henk 2006-03-29 23:50:41 PST
The widths of the textfields "Apple ID" and "Password" are incorrect.

The regression was intruduced starting with nightly r13567
Comment 1 Darin Adler 2006-03-31 23:05:59 PST
Actually, the widths of the text fields look fine to me (same as before). The problem seems to be the position of the text fields, not the widths.
Comment 2 Darin Adler 2006-04-02 21:02:19 PDT
Created attachment 7475 [details]
reduction, showing this is some sort of min/max width issue
Comment 3 Darin Adler 2006-04-02 21:23:30 PDT
I think the bug is caused by the fact that RenderTextField::calcMinMaxWidth sets the min/max width to a width based on the input element's size attribute even when the style has a width specified.
Comment 4 Darin Adler 2006-04-02 21:35:43 PDT
RenderTextField::calcMinMaxWidth is clearly wrong in at least a few ways. For example, it doesn't consider border or padding. It doesn't consider the values of any CSS properties (width, min-width, max-width).

What's annoying is that we probably want most of the logic from RenderBlock::calcMinMaxWidth, but we can't use it because it calls non-virtual calcInlineMinMaxWidth or calcBlockMinMaxWidth. We can look at RenderReplaced::calcMinMaxWidth for inspiration, I guess. Or maybe Hyatt already has a good technique in mind for this.
Comment 5 Maciej Stachowiak 2006-04-02 23:25:39 PDT
These are all text field regressions so they should all be P1.
Comment 6 Adele Peterson 2006-04-03 16:02:00 PDT
working on this now.
Comment 7 Darin Adler 2006-04-03 21:12:53 PDT
Assigning to Adele since she says she is working on this.
Comment 8 Adele Peterson 2006-04-04 10:09:01 PDT
Created attachment 7503 [details]
initial patch

Dave, can you take a look at this first cut of fixing RenderTextField::calcMinMaxWidth?
Comment 9 Dave Hyatt 2006-04-04 13:40:06 PDT
Comment on attachment 7503 [details]
initial patch

You shouldn't need these lines at all:

+    calcBlockMinMaxWidth();
+
+    if(m_maxWidth < m_minWidth) m_maxWidth = m_minWidth;

Other than that, I think this is right, although we probably need more tests of non-fixed widths on text fields (i.e., width:100%).