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: | P1 | Keywords: | Regression | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac (Intel) | ||||||||||
OS: | OS X 10.7 | ||||||||||
URL: | http://felixcloutier.com/documents/webkit-fileinput.html | ||||||||||
Attachments: |
|
Description
Félix Cloutier
2011-09-03 11:29:27 PDT
Created attachment 106265 [details]
An HTML page that demonstrates the problem
How recent is this? Can you try bisecting. I can't reproduce on Chromium using WebKit @94469. 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. It seems no one clears FileInputType::m_icon. This issue doesn't affect Chromium because Chromium doesn't support icons for file upload controls. Created attachment 106391 [details]
Patch
Created attachment 106392 [details]
Patch 2
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. Committed r94639: <http://trac.webkit.org/changeset/94639> 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. |