Bug 8072 - REGRESSION: text fields at connect.apple.com spill out of the containing box
Summary: REGRESSION: text fields at connect.apple.com spill out of the containing box
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Adele Peterson
URL: http://connect.apple.com/
Keywords: Regression
Depends on:
Reported: 2006-03-29 23:50 PST by Henk
Modified: 2006-04-04 15:20 PDT (History)
4 users (show)

See Also:

reduction, showing this is some sort of min/max width issue (1.52 KB, text/html)
2006-04-02 21:02 PDT, Darin Adler
no flags Details
initial patch (2.37 KB, patch)
2006-04-04 10:09 PDT, Adele Peterson
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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%).