Bug 8297 - REGRESSION: Input element extends outside of DIV element at http://www.macdock.com/
Summary: REGRESSION: Input element extends outside of DIV element at http://www.macdoc...
Status: VERIFIED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Adele Peterson
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2006-04-10 10:21 PDT by Chris Petersen
Modified: 2006-04-20 22:59 PDT (History)
3 users (show)

See Also:


Attachments
test case mentioned in description (236 bytes, text/html)
2006-04-17 19:01 PDT, Darin Adler
no flags Details
patch (40.35 KB, patch)
2006-04-19 17:38 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 Chris Petersen 2006-04-10 10:21:38 PDT
At http://www.macdock.com/, I noticed the Admin Email input text field extends outside of containing DIV. 
I have created a reduced test case of this page.

STEPS TO REPRODUCE
1. In TOT WebKit , open test case "macdock.html"
2.  Notice how wide the input field renders. It extends past the bordered DIV where the input element is contained.


HTML looks like this:

<div class="ad_box">
<input type="text"size="30" >
</div>

input {font-size:10px;}
.ad_box {border: solid;width:200px;}


The input element has been assigned size attribute in addition to font-size property.


REGRESSION
This issue only occurs with native text fields
Comment 1 Chris Petersen 2006-04-10 10:24:59 PDT
This issue has been filed as <rdar://problem/4508208>
Comment 2 Adele Peterson 2006-04-13 00:18:25 PDT
Note- this is no longer an issue on the live webpage.

I think this is due to the difference in width calculation for the size attribute.  While this calculation could use some tweaking (using average character width instead of a width of a particular character), I'm not sure if this qualifies as a P1 bug.  

It is different behavior, but we knew that some size calculations would be larger than they were before.  We were hoping that might account for some differences where our old size calculation was too small for the intended content.

Adding hyatt & beth in case they have any thoughts about what we should do here.
Comment 3 Darin Adler 2006-04-17 19:01:02 PDT
Created attachment 7789 [details]
test case mentioned in description
Comment 4 Darin Adler 2006-04-17 19:22:08 PDT
The problem here is caused by the fact that we are measuring a single character with "applyRunRounding" on. This results in a width that is rounded up to an integer, so the text field ends up too big. We need a way to measure with applyRunRounding false, or we can construct a string with a length == size and measure the entire string instead of measuring a single character and multiplying by the size.
Comment 5 Dave Hyatt 2006-04-18 18:30:12 PDT
Is that really all this is?  Firefox actually has a lot of room left over and doesn't even come close to spilling out of the div.  I find it hard to believe that run rounding alone could account for getting so much wider.

At any rate there are lots of cases that got better with the new text field where we were too small before, so it's not clear to me that this is a P1.
Comment 6 Dave Hyatt 2006-04-18 18:33:53 PDT
On the site in question it looks like the new text field is only 20px wider.  That's less than 1px per character, so I believe it's just run rounding. :)

Just measuring a string = to the size sounds good.
Comment 7 Adele Peterson 2006-04-19 17:38:42 PDT
Created attachment 7845 [details]
patch

Since there's no upper limit on the size (also true in IE & FF), we should probably turn off run rounding.  Here's a patch. (Darin- not sure if you already something for this- but I'm posting anyway)
Comment 8 Adele Peterson 2006-04-19 17:40:39 PDT
I meant to say:  because there's no upper limit for the size attr, we should turn off run rounding instead of constructing a string of the full size.
Comment 9 Dave Hyatt 2006-04-19 17:56:24 PDT
Comment on attachment 7845 [details]
patch

r=me
Comment 10 Adele Peterson 2006-04-19 17:57:36 PDT
Note- Dave and I discussed the font code that's missing from that patch.
Comment 11 Chris Petersen 2006-04-20 22:59:42 PDT
Verified with latest TOT Webkit build (r13990).