Bug 9316

Summary: REGRESSION: text field width shrinks on first keystroke
Product: WebKit Reporter: Tim Coulter <tim>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: jerrodblavos, jonathanjohnsson, mitz
Priority: P1 Keywords: HasReduction, InRadar, Regression
Version: 420+   
Hardware: Mac (PowerPC)   
OS: OS X 10.4   
URL: http://myspace.com/
Attachments:
Description Flags
Test case reduction
none
Disable the subtree optimization if the text field is floating
hyatt: review-
Return immediately from calcWidth() if this is the relayout-subtree root.
none
Return immediately from calcWidth() if this is the relayout-subtree root darin: review+

Tim Coulter
Reported 2006-06-05 11:30:16 PDT
I haven't noticed this with any other sites but MySpace. Tab or click into the "e-mail address" field like you're going to sign in, then start typing. The text field shrinks to 1 column (or less?), and doesn't seem to allow any input. Screenshot here: http://www.panic.com/~tim/myspace.png
Attachments
Test case reduction (941 bytes, text/html)
2006-06-06 11:45 PDT, jonathanjohnsson
no flags
Disable the subtree optimization if the text field is floating (26.94 KB, patch)
2006-06-17 12:14 PDT, mitz
hyatt: review-
Return immediately from calcWidth() if this is the relayout-subtree root. (27.22 KB, patch)
2006-06-23 16:40 PDT, mitz
no flags
Return immediately from calcWidth() if this is the relayout-subtree root (28.58 KB, patch)
2006-06-24 05:16 PDT, mitz
darin: review+
jonathanjohnsson
Comment 1 2006-06-05 15:40:50 PDT
The text field shrinks so you can't see what it contains, but it does allow for input. This is a regression from released Safari.
mitz
Comment 2 2006-06-05 15:47:33 PDT
Perhaps related to bug 9122
Alice Liu
Comment 3 2006-06-06 10:38:29 PDT
jonathanjohnsson
Comment 4 2006-06-06 11:45:51 PDT
Created attachment 8739 [details] Test case reduction
mitz
Comment 5 2006-06-06 11:53:20 PDT
(In reply to comment #4) > Created an attachment (id=8739) [edit] > Test case reduction Great reduction! The empty style element only serves the purpose of invalidating the doctype. If you delete both the empty style element and the doctype tag, the bug still occurs.
mitz
Comment 6 2006-06-15 14:50:51 PDT
I think this is a regression from the fix for bug 8969. The text field's containing block is not taking part in the layout, so its floating objects list contains the text field when the field's width is calculated, so that calculation is based on an incorrect lineWidth. It can be fixed by not doing subtree layout if the text field is floating, but perhaps there's a more subtle way to fix it.
jonathanjohnsson
Comment 7 2006-06-16 15:00:16 PDT
*** Bug 9474 has been marked as a duplicate of this bug. ***
mitz
Comment 8 2006-06-17 12:14:08 PDT
Created attachment 8888 [details] Disable the subtree optimization if the text field is floating
Darin Adler
Comment 9 2006-06-17 17:14:43 PDT
Hyatt, can you think of a better way to deal with this?
Dave Hyatt
Comment 10 2006-06-22 15:48:35 PDT
I don't understand this one. Why would you have to disable the subtree optimization?
Dave Hyatt
Comment 11 2006-06-23 04:09:36 PDT
Ok, I get the bug. Normally floating object lists are cleared during layout so that as a floating object does a layout, it can consume the appropriate amount of line width accounting for remaining space on the line. However, I think there's a better fix. If you are the subtree root then your width is never allowed to change. Therefore you can optimize out calcWidth and not call it at all if you are the subtree root. (It would be better I think to patch calcWidth itself to check if you are the subtree root, and if you are, it can just immediately return).
mitz
Comment 12 2006-06-23 16:40:36 PDT
Created attachment 8992 [details] Return immediately from calcWidth() if this is the relayout-subtree root. There are several calcWidth() implementation, but I think RenderBox's is the only one that's relevant, so I patched just it. I think the condition for when to return is the correct one, but I may have missed a more direct/obvious/efficient way to compute it.
Dave Hyatt
Comment 13 2006-06-23 16:43:17 PDT
Does the FrameView not know the root node of the subtree?   Seems like it could cache it and you could just grab it from the view.
mitz
Comment 14 2006-06-24 05:16:28 PDT
Created attachment 8997 [details] Return immediately from calcWidth() if this is the relayout-subtree root Using the layout root from the FrameView.
Darin Adler
Comment 15 2006-06-24 07:48:19 PDT
Comment on attachment 8997 [details] Return immediately from calcWidth() if this is the relayout-subtree root r=me
Alexey Proskuryakov
Comment 16 2006-06-24 08:47:19 PDT
Committed revision 15013.
Note You need to log in before you can comment on or make changes to this bug.