Bug 15671

Summary: [Transforms] The caret doesn't blink inside transformed content.
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Severity: Normal CC: eric, gregg, justin.garcia, mitz, simon.fraser
Priority: P2    
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on: 21942    
Bug Blocks: 15670, 18751, 22304, 19058    
Description Flags
WIP patch, makes caret painting use local coords
WIP patch 2
Final patch, changlog.
Testcase for arrow key navigation with transforms
Final patch, doesn't break arrow navigation hyatt: review+

Description Dave Hyatt 2007-10-24 13:21:32 PDT
Caret rects should be reworked to not rely on absolute position.
Comment 1 Simon Fraser (smfr) 2008-10-27 16:05:23 PDT
*** Bug 21907 has been marked as a duplicate of this bug. ***
Comment 2 Simon Fraser (smfr) 2008-10-27 16:05:30 PDT
RenderText::caretRect() calls absolutePositionForContent(), which fails to take
transforms into account.
Comment 3 Simon Fraser (smfr) 2008-11-07 17:28:20 PST
This is another case, like bug 21906, where someone is trying to draw an absolute rectangle, but there's a transform in the parent chain which changes the CTM. The caret drawing is going to have to be "more local".
Comment 4 Simon Fraser (smfr) 2008-11-09 11:15:49 PST
Created attachment 25002 [details]
WIP patch, makes caret painting use local coords
Comment 5 Simon Fraser (smfr) 2008-11-12 23:01:10 PST
Created attachment 25118 [details]
WIP patch 2

This patch has problems with scrolled textareas because the renderer used to compute the caret coords isn't necessarily the one calling paintCaret(); see RenderBlock::paintCaret().
Comment 6 Simon Fraser (smfr) 2008-11-14 23:33:56 PST
Created attachment 25182 [details]
Final patch, changlog.

Patch includes a new base for the one existing LayoutTest that shows a transformed caret. I can add more if appropriate.

All pixel tests pass with this patch.
Comment 7 Simon Fraser (smfr) 2008-11-16 18:18:45 PST
Created attachment 25204 [details]
Testcase for arrow key navigation with transforms
Comment 8 Simon Fraser (smfr) 2008-11-16 22:52:59 PST
Created attachment 25209 [details]
Final patch, doesn't break arrow navigation
Comment 9 Simon Fraser (smfr) 2008-11-16 22:56:06 PST
Filed bug 22305 on improving arrow navigation with transforms.
Comment 10 Dave Hyatt 2008-12-05 09:38:32 PST
Comment on attachment 25209 [details]
Final patch, doesn't break arrow navigation

+            // First compute a rect local to the rendere at the selection start

Typo: you meant "renderer"

"// because it's m_frame is always 0."

it's should be its

"+     , m_beginMinWidth(0)
+     , m_endMinWidth(0)"

This seems irrelevant to the patch?  Not that I'm complaining, just curious.

Comment 11 Simon Fraser (smfr) 2008-12-06 11:18:42 PST
(In reply to comment #10)
> "+     , m_beginMinWidth(0)
> +     , m_endMinWidth(0)"
> This seems irrelevant to the patch?  Not that I'm complaining, just curious.

Yes, it's not relevant to the caret changes, but just fixing some variable that I noticed during debugging were not initialized.
Comment 12 Simon Fraser (smfr) 2008-12-06 11:58:43 PST
	M	WebKit/mac/ChangeLog
	M	WebKit/mac/WebView/WebFrame.mm
	M	WebKit/qt/ChangeLog
	M	WebKit/qt/Api/qwebpage.cpp
	M	WebKit/gtk/webkit/webkitwebview.cpp
	M	WebKit/gtk/ChangeLog
	M	WebKit/win/ChangeLog
	M	WebKit/win/WebView.cpp
	M	WebCore/editing/mac/SelectionControllerMac.mm
	M	WebCore/editing/SelectionController.h
	M	WebCore/editing/DeleteSelectionCommand.cpp
	M	WebCore/editing/VisiblePosition.cpp
	M	WebCore/editing/SelectionController.cpp
	M	WebCore/editing/VisiblePosition.h
	M	WebCore/ChangeLog
	M	WebCore/WebCore.base.exp
	M	WebCore/page/Frame.h
	M	WebCore/page/AccessibilityRenderObject.cpp
	M	WebCore/page/Frame.cpp
	M	WebCore/html/HTMLElement.cpp
	M	WebCore/rendering/RenderObject.cpp
	M	WebCore/rendering/RenderFlow.h
	M	WebCore/rendering/RenderBox.h
	M	WebCore/rendering/RenderFlow.cpp
	M	WebCore/rendering/RenderSVGInlineText.h
	M	WebCore/rendering/RenderObject.h
	M	WebCore/rendering/RenderLayer.cpp
	M	WebCore/rendering/RenderSVGInlineText.cpp
	M	WebCore/rendering/RenderText.cpp
	M	WebCore/rendering/RenderBox.cpp
	M	WebCore/rendering/RenderBlock.cpp
	M	WebCore/rendering/RenderBlock.h
	M	WebCore/rendering/LayoutState.cpp
	M	WebCore/rendering/RenderText.h
	M	LayoutTests/platform/mac/fast/transforms/transformed-focused-text-input-expected.checksum
	A	LayoutTests/platform/mac/fast/transforms/transformed-caret-expected.png
	M	LayoutTests/platform/mac/fast/transforms/transformed-focused-text-input-expected.png
	A	LayoutTests/platform/mac/fast/transforms/transformed-caret-expected.checksum
	A	LayoutTests/platform/mac/fast/transforms/transformed-caret-expected.txt
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/transforms/transformed-caret.html
r39069 = 2b2b6ce0b2190e0531a3182661dd483b8202f0b9 (trunk)
Comment 13 Simon Fraser (smfr) 2008-12-06 17:08:06 PST
And one more pixel test updated:
Committed r39074
	M	LayoutTests/platform/mac/fast/forms/search-transformed-expected.png
	M	LayoutTests/platform/mac/fast/forms/search-transformed-expected.checksum
	M	LayoutTests/ChangeLog
r39074 = 75efff3244d98c9f0f7a2dc6000ce143bfa4a6a0 (trunk)
Comment 14 Gregg 2012-08-22 21:49:10 PDT
Has this really ben resolved? I'm still seeing exactly this issue in Chrome.
Comment 15 Simon Fraser (smfr) 2012-08-22 22:13:58 PDT
Please open a new bug with a testcase.
Comment 16 Simon Fraser (smfr) 2012-08-23 10:11:11 PDT
Bug 18751 covers an existing caret drawing problem with transforms.
Comment 17 Gregg 2012-08-23 10:43:20 PDT
True, but then this issue isn't really "RESOLVED FIXED". It's a bit misleading. Regardless, thanks for pointing me to the linked bug report. I'll keep an eye on it.
Comment 18 Gregg 2012-08-23 10:45:00 PDT
No, wait, that bug (18751) says that the caret blinking problem was split off into this one. Am I missing something here?
Comment 19 Gregg 2012-08-24 14:25:50 PDT
I have reproduced the issue in a jsfiddle. I'm using the isotope plugin in my project, so I used it in the jsfiddle to show this issue. Isotope uses the scale3d transform. When this transform is not used, the issue is not observed. To turn off transforms in isotope, simply add "transformsEnabled: false" to the passed options.


To see the issue, focus any textbox. The caret won't be blinking. Type and see that the caret still isn't blinking. Arrow key to the right and see that the caret doesn't move. Type again and see that the text shows up where the caret should be and also moves the caret into position.
Comment 20 Eric Seidel (no email) 2012-08-24 15:24:53 PDT
Caret blinking problems are usually repaint issues.  Could you please file a separate bug with a test case?  If you CC me, I'm happy to take a look.  Thanks!
Comment 21 Simon Fraser (smfr) 2012-08-27 12:44:13 PDT
*** Bug 94985 has been marked as a duplicate of this bug. ***
Comment 22 Simon Fraser (smfr) 2013-01-22 15:45:00 PST
*** Bug 94985 has been marked as a duplicate of this bug. ***