26471 2009-06-16 22:00:14 -0700 Redundant 'change' event for <input type=file> 2009-06-17 10:01:23 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 tkent tkent oldest_to_newest 126382 0 tkent 2009-06-16 22:00:14 -0700 The current implementation of FileChooser.cpp produces some redundant 'change' events in a case that the selected file is not changed. It makes a problem only in Chrome for now, and it is possible to resolve the problem by changing the Chrome code. However to change the WebKit code is safer and reasonable. See http://crbug.com/14162 126385 1 31399 tkent 2009-06-16 22:04:05 -0700 Created attachment 31399 Proposed patch 126395 2 31399 fishd 2009-06-16 23:37:42 -0700 Comment on attachment 31399 Proposed patch > Index: WebCore/ChangeLog ... > +2009-06-16 Kent Tamura <tkent@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. 126396 3 darin 2009-06-16 23:39:30 -0700 > + if (m_filenames.isEmpty() && filenames.isEmpty()) > + return; > + if (m_filenames.size() == 1 && filenames.size() == 1 && m_filenames[0] == filenames[0]) > + return; How about just: if (m_filenames == filenames) here? 126401 4 31403 tkent 2009-06-17 00:10:36 -0700 Created attachment 31403 Proposed patch (rev.2) Thank you for the comments. I have revised the patch. I confirmed "m_filenames == filenames" worked fine. 126482 5 fishd 2009-06-17 10:01:23 -0700 Landed as http://trac.webkit.org/changeset/44767 31399 2009-06-16 22:04:05 -0700 2009-06-17 00:10:36 -0700 Proposed patch chooser.patch text/plain 1776 tkent SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NDc1MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMDktMDYtMTYgIEtlbnQgVGFtdXJhICA8dGtlbnRAY2hyb21pdW0u b3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisJRG9uJ3QgZmly ZSByZWR1bmRhbnQgJ2NoYW5nZScgZXZlbnRzIGZvciBhIGZpbGUgdXBsb2FkIGZvcm0uCisKKyAg ICAgICAgV0FSTklORzogTk8gVEVTVCBDQVNFUyBBRERFRCBPUiBDSEFOR0VECisKKyAgICAgICAg KiBwbGF0Zm9ybS9GaWxlQ2hvb3Nlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpGaWxlQ2hvb3Nl cjo6Y2hvb3NlRmlsZXMpOiBEbyBub3RoaW5nIGlmIHRoZXJlIGFyZSBubyBleGlzdGluZyBzZWxl Y3RlZCBmaWxlcyBhbmQgbm8gaW5jb21pbmcgc2VsZWN0ZWQgZmlsZXMsIG9yIGlmIHRoZSBzaXpl IG9mIGJvdGggb2YgZXhpc3Rpbmcgc2VsZWN0ZWQgZmlsZXMgYW5kIGluY29taW5nIHNlbGVjdGVk IGZpbGVzIGlzIDEgYW5kIGZpbGVuYW1lcyBhcmUgaWRlbnRpY2FsLgorICAgICAgICAoV2ViQ29y ZTo6RmlsZUNob29zZXI6OmNob29zZUljb24pOiBSZXR1cm5zIDAgaWYgdGhlcmUgaXMgbm8gc2Vs ZWN0ZWQgZmlsZXMuCisKIDIwMDktMDYtMTYgIEJyaWFuIFdlaW5zdGVpbiAgPGJ3ZWluc3RlaW5A YXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEFkZWxlIFBldGVyc29uLgpJbmRleDog V2ViQ29yZS9wbGF0Zm9ybS9GaWxlQ2hvb3Nlci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9w bGF0Zm9ybS9GaWxlQ2hvb3Nlci5jcHAJKHJldmlzaW9uIDQ0NzQ4KQorKysgV2ViQ29yZS9wbGF0 Zm9ybS9GaWxlQ2hvb3Nlci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTcyLDYgKzcyLDEwIEBAIHZv aWQgRmlsZUNob29zZXI6OmNob29zZUZpbGUoY29uc3QgU3RyaW4KIAogdm9pZCBGaWxlQ2hvb3Nl cjo6Y2hvb3NlRmlsZXMoY29uc3QgVmVjdG9yPFN0cmluZz4mIGZpbGVuYW1lcykKIHsKKyAgICBp ZiAobV9maWxlbmFtZXMuaXNFbXB0eSgpICYmIGZpbGVuYW1lcy5pc0VtcHR5KCkpCisgICAgICAg IHJldHVybjsKKyAgICBpZiAobV9maWxlbmFtZXMuc2l6ZSgpID09IDEgJiYgZmlsZW5hbWVzLnNp emUoKSA9PSAxICYmIG1fZmlsZW5hbWVzWzBdID09IGZpbGVuYW1lc1swXSkKKyAgICAgICAgcmV0 dXJuOwogICAgIG1fZmlsZW5hbWVzID0gZmlsZW5hbWVzOwogICAgIG1faWNvbiA9IGNob29zZUlj b24oZmlsZW5hbWVzKTsKICAgICBpZiAobV9jbGllbnQpCkBAIC04NSw2ICs4OSw4IEBAIFBhc3NS ZWZQdHI8SWNvbj4gRmlsZUNob29zZXI6OmNob29zZUljb24KIAogUGFzc1JlZlB0cjxJY29uPiBG aWxlQ2hvb3Nlcjo6Y2hvb3NlSWNvbihWZWN0b3I8U3RyaW5nPiBmaWxlbmFtZXMpCiB7CisgICAg aWYgKGZpbGVuYW1lcy5pc0VtcHR5KCkpCisgICAgICAgIHJldHVybiAwOwogICAgIGlmIChmaWxl bmFtZXMuc2l6ZSgpID09IDEpCiAgICAgICAgIHJldHVybiBJY29uOjpjcmVhdGVJY29uRm9yRmls ZShmaWxlbmFtZXNbMF0pOwogICAgIHJldHVybiBJY29uOjpjcmVhdGVJY29uRm9yRmlsZXMoZmls ZW5hbWVzKTsK 31403 2009-06-17 00:10:36 -0700 2009-06-17 09:40:32 -0700 Proposed patch (rev.2) chooser-2.patch text/plain 1557 tkent SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NDc1MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTQgQEAKKzIwMDktMDYtMTYgIEtlbnQgVGFtdXJhICA8dGtlbnRAY2hyb21pdW0u b3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIERv bid0IGZpcmUgcmVkdW5kYW50ICdjaGFuZ2UnIGV2ZW50cyBmb3IgYSBmaWxlIHVwbG9hZCBmb3Jt LgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjY0NzEK KworICAgICAgICAqIHBsYXRmb3JtL0ZpbGVDaG9vc2VyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkZpbGVDaG9vc2VyOjpjaG9vc2VGaWxlcyk6IFN1cHByZXNzIGNoYW5nZSBldmVudCBpZiB0aGUg ZXhpc3Rpbmcgc2VsZWN0ZWQgZmlsZXMgYW5kIHRoZSBpbmNvbWluZyBzZWxlY3RlZCBmaWxlcyBh cmUgZXF1YWwuCisgICAgICAgIChXZWJDb3JlOjpGaWxlQ2hvb3Nlcjo6Y2hvb3NlSWNvbik6IFJl dHVybnMgMCBpZiB0aGVyZSBpcyBubyBzZWxlY3RlZCBmaWxlcy4KKwogMjAwOS0wNi0xNiAgQnJp YW4gV2VpbnN0ZWluICA8YndlaW5zdGVpbkBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQg YnkgQWRlbGUgUGV0ZXJzb24uCkluZGV4OiBXZWJDb3JlL3BsYXRmb3JtL0ZpbGVDaG9vc2VyLmNw cAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL0ZpbGVDaG9vc2VyLmNwcAkocmV2aXNp b24gNDQ3NDgpCisrKyBXZWJDb3JlL3BsYXRmb3JtL0ZpbGVDaG9vc2VyLmNwcAkod29ya2luZyBj b3B5KQpAQCAtNzIsNiArNzIsOCBAQCB2b2lkIEZpbGVDaG9vc2VyOjpjaG9vc2VGaWxlKGNvbnN0 IFN0cmluCiAKIHZvaWQgRmlsZUNob29zZXI6OmNob29zZUZpbGVzKGNvbnN0IFZlY3RvcjxTdHJp bmc+JiBmaWxlbmFtZXMpCiB7CisgICAgaWYgKG1fZmlsZW5hbWVzID09IGZpbGVuYW1lcykKKyAg ICAgICAgcmV0dXJuOwogICAgIG1fZmlsZW5hbWVzID0gZmlsZW5hbWVzOwogICAgIG1faWNvbiA9 IGNob29zZUljb24oZmlsZW5hbWVzKTsKICAgICBpZiAobV9jbGllbnQpCkBAIC04NSw2ICs4Nyw4 IEBAIFBhc3NSZWZQdHI8SWNvbj4gRmlsZUNob29zZXI6OmNob29zZUljb24KIAogUGFzc1JlZlB0 cjxJY29uPiBGaWxlQ2hvb3Nlcjo6Y2hvb3NlSWNvbihWZWN0b3I8U3RyaW5nPiBmaWxlbmFtZXMp CiB7CisgICAgaWYgKGZpbGVuYW1lcy5pc0VtcHR5KCkpCisgICAgICAgIHJldHVybiAwOwogICAg IGlmIChmaWxlbmFtZXMuc2l6ZSgpID09IDEpCiAgICAgICAgIHJldHVybiBJY29uOjpjcmVhdGVJ Y29uRm9yRmlsZShmaWxlbmFtZXNbMF0pOwogICAgIHJldHVybiBJY29uOjpjcmVhdGVJY29uRm9y RmlsZXMoZmlsZW5hbWVzKTsK