[webkit-reviews] review denied: [Bug 26471] Redundant 'change' event for <input type=file> : [Attachment 31399] Proposed patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied TAMURA, Kent
<tkent at chromium.org>'s request for review:
Bug 26471: Redundant 'change' event for <input type=file>
https://bugs.webkit.org/show_bug.cgi?id=26471

Attachment 31399: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=31399&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
> Index: WebCore/ChangeLog
...
> +2009-06-16  Kent Tamura  <tkent at chromium.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	Don't fire redundant 'change' events for a file upload form.

nit: please replace tabs with whitespaces


> +
> +	   WARNING: NO TEST CASES ADDED OR CHANGED

nit: you can delete this line.

it is also good practice to include a link to this bug in this ChangeLog entry.



> +	   * platform/FileChooser.cpp:
> +	   (WebCore::FileChooser::chooseFiles): Do nothing if there are no
existing selected files and no incoming selected files, or if the size of both
of existing selected files and incoming selected files is 1 and filenames are
identical.

nit: this can probably be shortened a bit.  it is good to mention what you
changed, but it
would probably be enough to say "suppress change event if an empty path is
chosen when
m_filenames is empty."	<-  just documents what you added


Otherwise, this change looks good to me.  I think this is good to add to
WebCore since it may similarly benefit other ports.