Bug 45940

Summary: REGRESSION (r67166): "Placeholder" text remains in input box after 2nd focus()
Product: WebKit Reporter: Joel <joel>
Component: HTML DOMAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Major CC: aestes, arv, commit-queue, dglazkov, mitz, paulirish, s+webkit, tkent
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 Joel 2010-09-16 19:13:30 PDT
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 Paul Irish 2010-09-20 20:27:31 PDT
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 Andy Estes 2010-09-24 14:20:40 PDT
Created attachment 68745 [details]
Reduction
Comment 3 Joel 2010-09-24 14:25:48 PDT
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 Andy Estes 2010-09-24 14:27:12 PDT
This regressed somewhere between r67129 and r67358.
Comment 5 Andy Estes 2010-09-24 16:04:27 PDT
Looks like this regressed in <http://trac.webkit.org/changeset/67166>.
Comment 6 Andy Estes 2010-09-24 16:18:53 PDT
(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 Kent Tamura 2010-09-26 21:21:00 PDT
Created attachment 68878 [details]
Patch
Comment 8 Kyle Fox 2010-09-28 15:24:11 PDT
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 Kent Tamura 2010-09-28 16:41:32 PDT
*** Bug 46757 has been marked as a duplicate of this bug. ***
Comment 10 mitz@webkit.org 2010-09-30 09:51:50 PDT
<rdar://problem/8497335>
Comment 11 sadrul 2010-10-01 08:36:41 PDT
Created attachment 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 Kent Tamura 2010-10-03 23:00:40 PDT
(In reply to comment #11)
> 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().

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 Sadrul Habib Chowdhury 2010-10-04 13:38:21 PDT
Created attachment 69675 [details]
Updated patch

Added the layouttests and changelogs from Kent's patch.
Comment 14 WebKit Commit Bot 2010-10-04 22:57:38 PDT
Comment on attachment 69675 [details]
Updated patch

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 Kent Tamura 2010-10-04 23:41:34 PDT
Comment on attachment 69675 [details]
Updated patch

I tried sadrul's patch locally and found it broke another test.
My patch should be necessary and sufficient.
Comment 16 Dimitri Glazkov (Google) 2010-10-05 19:21:34 PDT
Comment on attachment 69675 [details]
Updated patch

ok.
Comment 17 Dimitri Glazkov (Google) 2010-10-05 19:22:29 PDT
Comment on attachment 69675 [details]
Updated patch

doh.
Comment 18 Dimitri Glazkov (Google) 2010-10-05 19:23:10 PDT
Comment on attachment 68878 [details]
Patch

ok.
Comment 19 Kent Tamura 2010-10-05 21:36:48 PDT
(In reply to comment #7)
> Created an attachment (id=68878) [details]
> Patch

Committed this one as r69176.