REGRESSION (r23994): No caret is drawn after clicking a search field's placeholder text

• Component: New Bugs
• Status: RESOLVED FIXED

I noticed that sometimes (about half the time) when I click the search field on the right hand side of the page in the URL, the search field gets focus and I can type into it, but the caret is not showing.

[David Kilzer (:ddkilzer)]

I can't reproduce this with a local debug build of WebKit r24439 with Safari 3.0 (522.12) on Mac OS X 10.4.10 (8R218).  Is it the search field in the top nav bar, or the search field within the content of the page?

(In reply to comment #1)
> Is it the search field in the top
> nav bar, or the search field within the content of the page?

The one within the content of the page.

Reduction:
<input type="search" placeholder="Click this text">
Click the placeholder text, not the space to the right, to see the bug.

Regressed in <http://trac.webkit.org/projects/webkit/changeset/23994> (note that this is not the regression that was fixed in r24003).

The problem is that computing the target element lazily breaks when the target node is moved around during event dispatch. By the time handleMousePressEventSingleClick() sees the event and looks for the target node, it gets the renderer-less text node instead of the search field's inner div element.

The fact that this worked before r23994 is just a coincidence. In fact, if you try to cache the "target element" at MouseEventWithHitTestResults construction time, you also get it wrong, because the text node is still in the document at that point. It was really the gap between when the target element was cached and when the target node was computed -- a gap during which the text node had been removed from the tree -- that made it work prior to r23994.

While the old behavior is easy to reconstruct, the correct fix would be for the placeholder text never to be returned from nodeAtPoint. The fact that placeholder text is implemented using a text node should remain private to RenderTextControl.

[Matt Lilek]

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

Created attachment 15707 [details]
Patch idea

This works (without checking if the field is currently displaying placeholder text or not) because the inner text div cannot contain inline elements. Since positionForCoordinates is not patched, you can still select text etc.

Not directly related to the regression, I noticed that the behavior in TOT of drag and drop into a text field with placeholder text is funny: it draws a drop caret in the placeholder text. I am not sure what it should do.

Created attachment 15717 [details]
Patch, including change log and layout test

Perhaps not a very elegant solution, but the least intrusive that I could think of.

[Darin Adler]

Comment on attachment 15717 [details]
Patch, including change log and layout test

+    virtual ~RenderTextControlInnerBlock() { }

There's no need to declare that explicitly.

r=me

[Mark Rowe (bdash)]

<rdar://problem/5383841>

[Mark Rowe (bdash)]

Landed in r24842.

[Mark Rowe (bdash)]

r24850 was a followup fix for the Windows build.