Bug 11974

Summary: REGRESSION: Caret drawn over input when smaller than font size on initial focus
Product: WebKit Reporter: Matt Lilek <dev+webkit>
Component: FormsAssignee: Antti Koivisto <koivisto>
Status: RESOLVED FIXED    
Severity: Normal CC: koivisto, KwhiteRight
Priority: P1 Keywords: HasReduction, InRadar, Regression
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: data:text/html,<input type="text" style="height: 10px">
Attachments:
Description Flags
Set controlClip for non-search text controls
none
if caret is on a layer make sure it is painted by that layer
hyatt: review-
do a more generic test to avoid double paint hyatt: review+

Description Matt Lilek 2006-12-25 23:02:46 PST
When an input is smaller than the font size, the caret will be drawn over the focus ring until text is entered into it, after that the caret is properly clipped.
Comment 1 Mark Rowe (bdash) 2007-01-28 19:06:07 PST
<rdar://problem/4960258>
Comment 2 Antti Koivisto 2007-02-14 07:10:14 PST
Created attachment 13163 [details]
Set controlClip for non-search text controls

Set controlClip but don't enable it for search fields (which have m_innerBlock) since icons would get clipped away. Search fields have minimum height anyway.
Comment 3 Maciej Stachowiak 2007-02-14 10:01:22 PST
Comment on attachment 13163 [details]
Set controlClip for non-search text controls

r=me
Comment 4 Sam Weinig 2007-02-14 12:34:20 PST
Landed in r19626!
Comment 5 Antti Koivisto 2007-02-14 13:38:25 PST
Turns out fix is not good. It causes problems with some text fields (google!) so reopening and reverting the patch.
Comment 6 Sam Weinig 2007-02-14 14:43:53 PST
Comment on attachment 13163 [details]
Set controlClip for non-search text controls

Removing review flag so that this does not get landed accidentally.
Comment 7 Antti Koivisto 2007-02-18 13:30:59 PST
Created attachment 13229 [details]
if caret is on a layer make sure it is painted by that layer

Actual problem is that the caret is painted twice, once by the associated renderer and another time by renderer's containing block. Painting done by the containing block won't respect clipping established by the renderer.

I'm not actually sure why it is ever necessary to paint caret in both the associated renderer and the containing block. The patch just fixes a case where this double painting actually causes problems.
Comment 8 Dave Hyatt 2007-02-18 14:30:37 PST
Comment on attachment 13229 [details]
if caret is on a layer make sure it is painted by that layer

Seems like we should just let the block that the caret is in always paint the renderer.  I do not understand how a double painting situation is occurring and think we should stop that rather than adding a layer check that should not be necessary.
Comment 9 Antti Koivisto 2007-02-18 14:53:49 PST
Created attachment 13230 [details]
do a more generic test to avoid double paint
Comment 10 Dave Hyatt 2007-02-19 01:17:20 PST
Comment on attachment 13230 [details]
do a more generic test to avoid double paint

I'd probably ask isBlockFlow first (since it's quicker than containingBlock()).

r=me!
Comment 11 Dave Hyatt 2007-02-19 01:23:17 PST
Disregard my comment.  I'm wrong about that ordering being better.

Comment 12 Antti Koivisto 2007-02-19 01:33:12 PST
r19706
Comment 13 mitz 2007-02-19 16:20:35 PST
The fix caused a regression where sometimes the caret isn't painted at all. See for example <http://build.webkit.org/results/post-commit-pixel-powerpc-mac-os-x/3675/editing/pasteboard/4641033-diffs.html>. You can open the test in Safari and see that when the caret is at the very end of the contenteditable div, after the select, it does not paint.