Bug 67567

Summary:

REGRESSION (Safari 5.1 - ToT): File input retains its file icon when the value is reset

Product:

WebKit

Reporter:

Félix Cloutier <felixcca>

Component:

Layout and Rendering

Assignee:

Kent Tamura <tkent>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dglazkov, haraken, jonlee, tkent

Priority:

Keywords:

Regression

Version:

528+ (Nightly build)

Hardware:

Mac (Intel)

OS:

OS X 10.7

URL:

http://felixcloutier.com/documents/webkit-fileinput.html

Attachments:

Description	Flags
An HTML page that demonstrates the problem	none
Patch	none
Patch 2	dglazkov: review+

Félix Cloutier

Reported 2011-09-03 11:29:27 PDT

Empty file inputs visually consist of a button and the text 'no file selected'. File inputs with a value, instead, have a button, a file icon, and a file label. However, when an <input type="file"> is reset through Javascript (input.value = ''), the file label reverts to 'no file selected', but the file icon stays there.

Attachments
An HTML page that demonstrates the problem (597 bytes, text/html) 2011-09-03 11:35 PDT, Félix Cloutier	no flags	Details
Patch (15.14 KB, patch) 2011-09-05 23:28 PDT, Kent Tamura	no flags	Details Formatted Diff Diff
Patch 2 (15.10 KB, patch) 2011-09-05 23:30 PDT, Kent Tamura	dglazkov: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Félix Cloutier

Comment 1 2011-09-03 11:35:11 PDT

Created attachment 106265 [details] An HTML page that demonstrates the problem

Dimitri Glazkov (Google)

Comment 2 2011-09-05 08:31:21 PDT

How recent is this? Can you try bisecting. I can't reproduce on Chromium using WebKit @94469.

Félix Cloutier

Comment 3 2011-09-05 11:14:50 PDT

It doesn't happen with the version of Safari that ships with Mac OS Lion (5.1, 7534.48.3), but it happens with r94508, again using Safari. I'm sorry, I can't really be any more precise.

Kent Tamura

Comment 4 2011-09-05 19:28:39 PDT

It seems no one clears FileInputType::m_icon. This issue doesn't affect Chromium because Chromium doesn't support icons for file upload controls.

Kent Tamura

Comment 5 2011-09-05 23:28:54 PDT

Created attachment 106391 [details] Patch

Kent Tamura

Comment 6 2011-09-05 23:30:06 PDT

Created attachment 106392 [details] Patch 2

Dimitri Glazkov (Google)

Comment 7 2011-09-06 08:46:20 PDT

Comment on attachment 106392 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=106392&action=review Thanks for fixing this -- and the fix is right. I have a small naming nit. > Source/WebCore/html/HTMLInputElement.cpp:1127 > +void HTMLInputElement::setValueInternal(const String& sanitizedValue, bool sendChangeEvent) This should be something like valueChanged.

Kent Tamura

Comment 8 2011-09-06 22:32:59 PDT

Committed r94639: <http://trac.webkit.org/changeset/94639>

Kent Tamura

Comment 9 2011-09-06 22:37:22 PDT

Comment on attachment 106392 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=106392&action=review >> Source/WebCore/html/HTMLInputElement.cpp:1127 >> +void HTMLInputElement::setValueInternal(const String& sanitizedValue, bool sendChangeEvent) > > This should be something like valueChanged. I don't agree with it because - The value is changed in this function. "valueChanged" sounds like "didChangeValue". - InputType already has valueChanged(), of which purpose is different from this. So, I landed this without changes.

Note You need to log in before you can comment on or make changes to this bug.