Bug 25277 - If there is no line wrapping, a caret can leave its containing block but it shouldn't leave its root line box
Summary: If there is no line wrapping, a caret can leave its containing block but it s...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Justin Garcia
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2009-04-17 16:15 PDT by Justin Garcia
Modified: 2009-04-17 19:00 PDT (History)
2 users (show)

See Also:

Attachments
patch (5.17 KB, patch)
2009-04-17 16:50 PDT, Justin Garcia 		no flags Details | Formatted Diff | Diff
patch (7.80 KB, patch)
2009-04-17 17:38 PDT, Justin Garcia 		mitz: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Garcia 2009-04-17 16:15:51 PDT 
If there is no line wrapping, a caret can leave its containing block but it shouldn't leave its root line box.  

Otherwise I believe it can be clipped by the containing blocks outline.  Test case forthcoming.

<rdar://problem/6800956>
Comment 1 Justin Garcia 2009-04-17 16:50:54 PDT 
Created attachment 29593 [details]
patch
Comment 2 mitz 2009-04-17 17:02:58 PDT 
Comment on attachment 29593 [details]
patch

You should add a reference to the test to the WebCore ChangeLog.

I don't think this covers all cases. Not all textAlign() values that are not LEFT should behave the same, and I think in most cases (maybe even for LEFT and RIGHT) the behavior also needs to take style()->direction() into account, because that's how we decide on which side of the block the overflow goes. See the logic in RenderBlock::computeHorizontalPositionsForLine().
Comment 3 Justin Garcia 2009-04-17 17:04:22 PDT 
(In reply to comment #2)
> Not all textAlign() values that are not LEFT should behave the same

oops!  will fix...
Comment 4 Justin Garcia 2009-04-17 17:38:28 PDT 
Created attachment 29597 [details]
patch

patch with new test case to cover the case that dan mentioned, where the text-align is right but the direction is LTR.  In that case we spill over to the right off the block like we do in other direction:LTR cases.
Comment 5 mitz 2009-04-17 18:31:14 PDT 
Comment on attachment 29597 [details]
patch

r=me
There's a problem with this and earlier caret painting patches in that the difference between correct and incorrect behavior in the pixel test is below our detection threshold. I am not sure what to do about it (maybe use the Mac SPI that's exposed via DRT for getting caret rects?), but addressing that is not part of this bug.
Comment 6 Simon Fraser (smfr) 2009-04-17 18:33:19 PDT 
Maybe caretWidth should be settable at runtime via a CSS property, or DRT API, and we can make it really fat?
Comment 7 mitz 2009-04-17 18:35:48 PDT 
I don't think we want a CSS property for it, but maybe a WebView SPI.
Comment 8 Justin Garcia 2009-04-17 18:38:16 PDT 
I'll correct the test description to say that the problem is that the caret paints over the focus halo a bit (rather than getting clipped).
Comment 9 Justin Garcia 2009-04-17 18:54:25 PDT 
http://trac.webkit.org/changeset/42634
Comment 10 Justin Garcia 2009-04-17 19:00:29 PDT 
and http://trac.webkit.org/changeset/42635