Bug 45940

Summary: REGRESSION (r67166): "Placeholder" text remains in input box after 2nd focus()
Product: WebKit Reporter: Joel <joel@bytestudios.com>
Component: HTML DOMAssignee: Kent Tamura <tkent@chromium.org>
Status: RESOLVED FIXED    
Severity: Major CC: aestes@apple.com, arv@chromium.org, commit-queue@webkit.org, dglazkov@chromium.org, mitz@webkit.org, paulirish@chromium.org, s+webkit@sidneysm.com, tkent@chromium.org
Priority: P1 Keywords: HasReduction, InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Macintosh Intel   
OS: Mac OS X 10.6   
URL: http://le.bytestudios.com/1.php
Attachments:
Description Flags
Reduction
none
Patch
dglazkov: review+
Alternate fix
none
Updated patch dglazkov: review-, commit-queue: commit‑queue-

Description From 2010-09-16 19:13:30 PST
http://le.bytestudios.com/1.php

focus and blur multiple times to see it happen

noticed this very recently in the latest nightly, safari 5 works as expected.
------- Comment #1 From 2010-09-20 20:27:31 PST -------
Demo page: http://www.thecssninja.com/demo/placeholder_issue/

chromium ticket: http://code.google.com/p/chromium/issues/detail?id=56226

repro in latest chromium build and webkit nightly.
------- Comment #2 From 2010-09-24 14:20:40 PST -------
Created an attachment (id=68745) [details]
Reduction
------- Comment #3 From 2010-09-24 14:25:48 PST -------
This might break some w3c guideline. but wouldn't it be nice to dim the placeholder text, instead of completely remove it on focus? that way you still know what you need to enter when you tab. keypress would clear the placeholder completely
------- Comment #4 From 2010-09-24 14:27:12 PST -------
This regressed somewhere between r67129 and r67358.
------- Comment #5 From 2010-09-24 16:04:27 PST -------
Looks like this regressed in <http://trac.webkit.org/changeset/67166>.
------- Comment #6 From 2010-09-24 16:18:53 PST -------
(In reply to comment #3)
> This might break some w3c guideline. but wouldn't it be nice to dim the placeholder text, instead of completely remove it on focus? that way you still know what you need to enter when you tab. keypress would clear the placeholder completely

From <http://dev.w3.org/html5/spec/Overview.html#attr-input-placeholder>:

"User agents should present this hint to the user, after having stripped line breaks from it, when the element's value is the empty string and the control is not focused (e.g. by displaying it inside a blank unfocused control)."

Clearing the placeholder text on focus sounds like the right thing to do based on the spec. This is off the topic of this bug, but you could probably accomplish what you describe by listening to focus and keydown events in JavaScript.
------- Comment #7 From 2010-09-26 21:21:00 PST -------
Created an attachment (id=68878) [details]
Patch
------- Comment #8 From 2010-09-28 15:24:11 PST -------
I just noticed this behaviour with an <input> with autofocus and filed a bug (https://bugs.webkit.org/show_bug.cgi?id=46757) before finding this ticket.

It's pretty much the same issue as this, however.
------- Comment #9 From 2010-09-28 16:41:32 PST -------
*** Bug 46757 has been marked as a duplicate of this bug. ***
------- Comment #10 From 2010-09-30 09:51:50 PST -------
<rdar://problem/8497335>
------- Comment #11 From 2010-10-01 08:36:41 PST -------
Created an attachment (id=69469) [details]
Alternate fix

The attached patch is an alternate solution. Since the check before setting the text value in the HTMLInputElement is to ensure that the value is 'acceptable', the patch checks for the hasUnacceptableValue() instead of formControlValueMatchesRenderer().

#16465 may also be this issue.
------- Comment #12 From 2010-10-03 23:00:40 PST -------
(In reply to comment #11)
> Created an attachment (id=69469) [details] [details]
> Alternate fix
> 
> The attached patch is an alternate solution. Since the check before setting the text value in the HTMLInputElement is to ensure that the value is 'acceptable', the patch checks for the hasUnacceptableValue() instead of formControlValueMatchesRenderer().

Your patch looks to have good readability.  Would you add ChangeLog and copy the test from my patch please?

> #16465 may also be this issue.

I don't think so.
------- Comment #13 From 2010-10-04 13:38:21 PST -------
Created an attachment (id=69675) [details]
Updated patch

Added the layouttests and changelogs from Kent's patch.
------- Comment #14 From 2010-10-04 22:57:38 PST -------
(From update of attachment 69675 [details])
Rejecting patch 69675 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 69675]" exit_code: 2
Cleaning working directory
Updating working directory
Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=69675&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=45940&ctype=xml
Processing 1 patch from 1 bug.
Processing patch 69675 from bug 45940.
Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Darin Adler', u'--force']" exit_code: 9

Full output: http://queues.webkit.org/results/4172080
------- Comment #15 From 2010-10-04 23:41:34 PST -------
(From update of attachment 69675 [details])
I tried sadrul's patch locally and found it broke another test.
My patch should be necessary and sufficient.
------- Comment #16 From 2010-10-05 19:21:34 PST -------
(From update of attachment 69675 [details])
ok.
------- Comment #17 From 2010-10-05 19:22:29 PST -------
(From update of attachment 69675 [details])
doh.
------- Comment #18 From 2010-10-05 19:23:10 PST -------
(From update of attachment 68878 [details])
ok.
------- Comment #19 From 2010-10-05 21:36:48 PST -------
(In reply to comment #7)
> Created an attachment (id=68878) [details] [details]
> Patch

Committed this one as r69176.