WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 14653
REGRESSION (
r23994
): No caret is drawn after clicking a search field's placeholder text
https://bugs.webkit.org/show_bug.cgi?id=14653
Summary
REGRESSION (r23994): No caret is drawn after clicking a search field's placeh...
mitz
Reported
2007-07-18 00:39:31 PDT
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.
Attachments
Patch idea
(1.64 KB, patch)
2007-07-27 11:10 PDT
,
mitz
no flags
Details
Formatted Diff
Diff
Patch, including change log and layout test
(5.02 KB, patch)
2007-07-28 04:40 PDT
,
mitz
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1
2007-07-18 23:06:58 PDT
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?
mitz
Comment 2
2007-07-18 23:12:32 PDT
(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.
mitz
Comment 3
2007-07-19 12:43:01 PDT
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.
mitz
Comment 4
2007-07-19 14:47:36 PDT
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
Comment 5
2007-07-27 00:25:50 PDT
***
Bug 14775
has been marked as a duplicate of this bug. ***
mitz
Comment 6
2007-07-27 11:10:39 PDT
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.
mitz
Comment 7
2007-07-28 04:40:55 PDT
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 8
2007-07-30 11:51:59 PDT
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)
Comment 9
2007-08-03 07:30:59 PDT
<
rdar://problem/5383841
>
Mark Rowe (bdash)
Comment 10
2007-08-03 07:34:10 PDT
Landed in
r24842
.
Mark Rowe (bdash)
Comment 11
2007-08-03 11:51:57 PDT
r24850
was a followup fix for the Windows build.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug