Bug 15671 – [Transforms] The caret doesn't blink inside transformed content.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dave Hyatt]

Caret rects should be reworked to not rely on absolute position.

[Simon Fraser (smfr)]

*** Bug 21907 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

RenderText::caretRect() calls absolutePositionForContent(), which fails to take
transforms into account.

[Simon Fraser (smfr)]

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".

[Simon Fraser (smfr)]

Created attachment 25002 [details]
WIP patch, makes caret painting use local coords

[Simon Fraser (smfr)]

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().

[Simon Fraser (smfr)]

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.

[Simon Fraser (smfr)]

Created attachment 25204 [details]
Testcase for arrow key navigation with transforms

[Simon Fraser (smfr)]

Created attachment 25209 [details]
Final patch, doesn't break arrow navigation

[Simon Fraser (smfr)]

Filed bug 22305 on improving arrow navigation with transforms.

[Dave Hyatt]

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.

r=me

[Simon Fraser (smfr)]

(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.

[Simon Fraser (smfr)]

	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)

[Simon Fraser (smfr)]

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)

[Gregg]

Has this really ben resolved? I'm still seeing exactly this issue in Chrome.

[Simon Fraser (smfr)]

Please open a new bug with a testcase.

[Simon Fraser (smfr)]

Bug 18751 covers an existing caret drawing problem with transforms.

[Gregg]

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.

[Gregg]

No, wait, that bug (18751) says that the caret blinking problem was split off into this one. Am I missing something here?

[Gregg]

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.

http://jsfiddle.net/5Rqv8/

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.

[Eric Seidel (no email)]

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!

[Simon Fraser (smfr)]

*** Bug 94985 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 94985 has been marked as a duplicate of this bug. ***