38014 2010-04-22 16:59:51 -0700 [chromium] Autofill ignores forms without 'action' attribute 2010-04-27 23:00:17 -0700 1 1 1 Unclassified WebKit WebKit API 528+ (Nightly build) All OS X 10.5 RESOLVED FIXED P2 Normal --- 1 jens jens commit-queue levin webkit.review.bot oldest_to_newest 216163 0 jens 2010-04-22 16:59:51 -0700 Chrome's password autofill doesn't handle forms where the <form> element has no 'action' attribute. http://code.google.com/p/chromium/issues/detail?id=29513 The bug is in Chrome's WebKit API implementation, specifically WebPasswordFormData. When it derives the form's actual action URL it ends up with a null URL if there's no action attribute, when instead it should default to the same URL as the page. And when it has a null URL it assumes the form is invalid and ignores it for autofill purposes. 216167 1 54110 jens 2010-04-22 17:03:36 -0700 Created attachment 54110 patch 216168 2 webkit.review.bot 2010-04-22 17:06:39 -0700 Attachment 54110 did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/chromium/src/WebPasswordFormData.cpp:167: One space before end of line comments [whitespace/comments] [5] Total errors found: 1 in 1 files If any of these errors are false positives, please file a bug against check-webkit-style. 216191 3 54114 jens 2010-04-22 17:51:05 -0700 Created attachment 54114 now, with less whitespace OK, WebKit only wants one space before a comment. Here it is. FML. 216521 4 54184 jens 2010-04-23 13:01:55 -0700 Created attachment 54184 now with changelog entry!! Sorry, forgot the changelog entry. I haven't touched WebKit in a few months and am getting rusty ... 217833 5 54184 eric 2010-04-27 12:11:35 -0700 Comment on attachment 54184 now with changelog entry!! Autosave code is outside the scope of webkit historically. However is Chromium's in webkit's repo? Either way, how do we test this? 217834 6 54184 eric 2010-04-27 12:12:42 -0700 Comment on attachment 54184 now with changelog entry!! Also, your ChangeLog is wrong. You should leave the "Reviewed by" line as-is, and the scripts will automatically fill in the reviewer. it's confusing, but that's how it works. The ChangeLog shoudl also explain what testing there is or why testing is impossible. See: http://webkit.org/coding/contributing.html 217848 7 54444 jens 2010-04-27 12:32:04 -0700 Created attachment 54444 updated changelog entry OK, fixed the reviewer name glitch and added info about (the lack of) tests. 217852 8 54444 eric 2010-04-27 12:37:20 -0700 Comment on attachment 54444 updated changelog entry Thank you. The test explanation update is helpful. One more question though: Why fix this here, by mapping the null string to "" instead of changing completeURL()? 217861 9 jens 2010-04-27 12:51:24 -0700 FrameLoader::completeURL is just a call-through wrapper for Document::completeURL), which explicitly states in a comment, "Always return a null URL when passed a null string." So this behavior appears to be intentional, and it seems likely that something would probably break, possibly in very subtle ways, if I changed it. It just happens that _in this particular case_ a missing 'action' attribute in an HTML <form> element defaults to the document's URL. 218103 10 54444 eric 2010-04-27 17:57:18 -0700 Comment on attachment 54444 updated changelog entry OK. 218215 11 54444 commit-queue 2010-04-27 23:00:12 -0700 Comment on attachment 54444 updated changelog entry Clearing flags on attachment: 54444 Committed r58382: <http://trac.webkit.org/changeset/58382> 218216 12 commit-queue 2010-04-27 23:00:17 -0700 All reviewed patches have been landed. Closing bug. 54110 2010-04-22 17:03:36 -0700 2010-04-22 17:51:05 -0700 patch textmate stdin F88KHN.txt text/plain 768 jens ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViUGFzc3dvcmRGb3JtRGF0YS5jcHAg Yi9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYlBhc3N3b3JkRm9ybURhdGEuY3BwCmluZGV4IDY0YjE3 NTQuLmM1MzNjMjAgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViUGFzc3dvcmRG b3JtRGF0YS5jcHAKKysrIGIvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJQYXNzd29yZEZvcm1EYXRh LmNwcApAQCAtMTYyLDcgKzE2MiwxMSBAQCBXZWJQYXNzd29yZEZvcm1EYXRhOjpXZWJQYXNzd29y ZEZvcm1EYXRhKGNvbnN0IFdlYkZvcm1FbGVtZW50JiB3ZWJGb3JtKQogICAgIEtVUkwgZnVsbE9y aWdpbihQYXJzZWRVUkxTdHJpbmcsIGZvcm0tPmRvY3VtZW50KCktPmRvY3VtZW50VVJJKCkpOwog CiAgICAgLy8gQ2FsY3VsYXRlIHRoZSBjYW5vbmljYWwgYWN0aW9uIFVSTAotICAgIEtVUkwgZnVs bEFjdGlvbiA9IGZyYW1lLT5sb2FkZXIoKS0+Y29tcGxldGVVUkwoZm9ybS0+YWN0aW9uKCkpOwor ICAgIFN0cmluZyBhY3Rpb24gPSBmb3JtLT5hY3Rpb24oKTsKKyAgICBpZiAoYWN0aW9uLmlzTnVs bCgpKQorICAgICAgICBhY3Rpb24gPSAiIjsgICAgICAgIC8vIG1pc3NpbmcgJ2FjdGlvbicgYXR0 cmlidXRlIGltcGxpZXMgY3VycmVudCBVUkwKKworICAgIEtVUkwgZnVsbEFjdGlvbiA9IGZyYW1l LT5sb2FkZXIoKS0+Y29tcGxldGVVUkwoYWN0aW9uKTsKICAgICBpZiAoIWZ1bGxBY3Rpb24uaXNW YWxpZCgpKQogICAgICAgICByZXR1cm47CiAK 54114 2010-04-22 17:51:05 -0700 2010-04-23 13:02:15 -0700 now, with less whitespace textmate stdin Y8iyl6.txt text/plain 759 jens ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViUGFzc3dvcmRGb3JtRGF0YS5jcHAg Yi9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYlBhc3N3b3JkRm9ybURhdGEuY3BwCmluZGV4IDY0YjE3 NTQuLmViMjMwZDUgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViUGFzc3dvcmRG b3JtRGF0YS5jcHAKKysrIGIvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJQYXNzd29yZEZvcm1EYXRh LmNwcApAQCAtMTYyLDcgKzE2MiwxMCBAQCBXZWJQYXNzd29yZEZvcm1EYXRhOjpXZWJQYXNzd29y ZEZvcm1EYXRhKGNvbnN0IFdlYkZvcm1FbGVtZW50JiB3ZWJGb3JtKQogICAgIEtVUkwgZnVsbE9y aWdpbihQYXJzZWRVUkxTdHJpbmcsIGZvcm0tPmRvY3VtZW50KCktPmRvY3VtZW50VVJJKCkpOwog CiAgICAgLy8gQ2FsY3VsYXRlIHRoZSBjYW5vbmljYWwgYWN0aW9uIFVSTAotICAgIEtVUkwgZnVs bEFjdGlvbiA9IGZyYW1lLT5sb2FkZXIoKS0+Y29tcGxldGVVUkwoZm9ybS0+YWN0aW9uKCkpOwor ICAgIFN0cmluZyBhY3Rpb24gPSBmb3JtLT5hY3Rpb24oKTsKKyAgICBpZiAoYWN0aW9uLmlzTnVs bCgpKQorICAgICAgICBhY3Rpb24gPSAiIjsgLy8gbWlzc2luZyAnYWN0aW9uJyBhdHRyaWJ1dGUg aW1wbGllcyBjdXJyZW50IFVSTAorICAgIEtVUkwgZnVsbEFjdGlvbiA9IGZyYW1lLT5sb2FkZXIo KS0+Y29tcGxldGVVUkwoYWN0aW9uKTsKICAgICBpZiAoIWZ1bGxBY3Rpb24uaXNWYWxpZCgpKQog ICAgICAgICByZXR1cm47CiAK 54184 2010-04-23 13:01:55 -0700 2010-04-27 12:32:04 -0700 now with changelog entry!! textmate stdin zN4D69.txt text/plain 1380 jens ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9XZWJLaXQvY2hyb21pdW0v Q2hhbmdlTG9nCmluZGV4IGVkNDNmZmYuLmNiYmNkZDcgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwpAQCAtMSwzICsx LDEyIEBACisyMDEwLTA0LTIzICBKZW5zIEFsZmtlICA8c25lakBjaHJvbWl1bS5vcmc+CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIQorICAgICAgICBGaXggYnVnIHRoYXQgcHJldmVudHMg YXV0b3NhdmUgb2YgZm9ybXMgd2l0aG91dCBhY3Rpb24gYXR0cmlidXRlCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zODAxNAorCisgICAgICAgICogc3Jj L1dlYlBhc3N3b3JkRm9ybURhdGEuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJQYXNzd29yZEZv cm1EYXRhOjpXZWJQYXNzd29yZEZvcm1EYXRhKTogU2V0IGNvcnJlY3QgVVJMIGlmIGFjdGlvbiBp cyBOVUxMCisKIDIwMTAtMDQtMjEgIFNoZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5ib3RAZ21h aWwuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHI1ODAyOC4KZGlmZiAt LWdpdCBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViUGFzc3dvcmRGb3JtRGF0YS5jcHAgYi9XZWJL aXQvY2hyb21pdW0vc3JjL1dlYlBhc3N3b3JkRm9ybURhdGEuY3BwCmluZGV4IDY0YjE3NTQuLmVi MjMwZDUgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViUGFzc3dvcmRGb3JtRGF0 YS5jcHAKKysrIGIvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJQYXNzd29yZEZvcm1EYXRhLmNwcApA QCAtMTYyLDcgKzE2MiwxMCBAQCBXZWJQYXNzd29yZEZvcm1EYXRhOjpXZWJQYXNzd29yZEZvcm1E YXRhKGNvbnN0IFdlYkZvcm1FbGVtZW50JiB3ZWJGb3JtKQogICAgIEtVUkwgZnVsbE9yaWdpbihQ YXJzZWRVUkxTdHJpbmcsIGZvcm0tPmRvY3VtZW50KCktPmRvY3VtZW50VVJJKCkpOwogCiAgICAg Ly8gQ2FsY3VsYXRlIHRoZSBjYW5vbmljYWwgYWN0aW9uIFVSTAotICAgIEtVUkwgZnVsbEFjdGlv biA9IGZyYW1lLT5sb2FkZXIoKS0+Y29tcGxldGVVUkwoZm9ybS0+YWN0aW9uKCkpOworICAgIFN0 cmluZyBhY3Rpb24gPSBmb3JtLT5hY3Rpb24oKTsKKyAgICBpZiAoYWN0aW9uLmlzTnVsbCgpKQor ICAgICAgICBhY3Rpb24gPSAiIjsgLy8gbWlzc2luZyAnYWN0aW9uJyBhdHRyaWJ1dGUgaW1wbGll cyBjdXJyZW50IFVSTAorICAgIEtVUkwgZnVsbEFjdGlvbiA9IGZyYW1lLT5sb2FkZXIoKS0+Y29t cGxldGVVUkwoYWN0aW9uKTsKICAgICBpZiAoIWZ1bGxBY3Rpb24uaXNWYWxpZCgpKQogICAgICAg ICByZXR1cm47CiAK 54444 2010-04-27 12:32:04 -0700 2010-04-27 23:00:11 -0700 updated changelog entry textmate stdin At0Dv5.txt text/plain 1574 jens ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9XZWJLaXQvY2hyb21pdW0v Q2hhbmdlTG9nCmluZGV4IDc5MTcwODcuLmU2NGVlMWUgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwpAQCAtMSwzICsx LDE1IEBACisyMDEwLTA0LTI3ICBKZW5zIEFsZmtlICA8c25lakBjaHJvbWl1bS5vcmc+CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW2Nocm9taXVtXSBG aXggYnVnIHRoYXQgcHJldmVudHMgYXV0b3NhdmUgb2YgZm9ybXMgd2l0aG91dCBhY3Rpb24gYXR0 cmlidXRlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0z ODAxNAorICAgICAgICBDaGFuZ2UgaXMgaW4gdGhlIENocm9taXVtIFdlYktpdCBBUEkgc28gaXQg Y2FuJ3QgYmUgdGVzdGVkIHdpdGggYSBKUy1iYXNlZCBsYXlvdXQgdGVzdC4KKyAgICAgICAgSXQg bmVlZHMgYSBuYXRpdmUgdW5pdCB0ZXN0LCB3aGljaCB3b3VsZCBnbyBpbiB0aGUgQ2hyb21pdW0g cmVwbywgbm90IFdlYktpdC4KKworICAgICAgICAqIHNyYy9XZWJQYXNzd29yZEZvcm1EYXRhLmNw cDoKKyAgICAgICAgKFdlYktpdDo6V2ViUGFzc3dvcmRGb3JtRGF0YTo6V2ViUGFzc3dvcmRGb3Jt RGF0YSk6IFNldCBjb3JyZWN0IFVSTCBpZiBhY3Rpb24gaXMgTlVMTAorCiAyMDEwLTA0LTI2ICBK b2huIEFiZC1FbC1NYWxlayAgPGphbUBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQg YnkgRGFyaW4gRmlzaGVyLgpkaWZmIC0tZ2l0IGEvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJQYXNz d29yZEZvcm1EYXRhLmNwcCBiL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViUGFzc3dvcmRGb3JtRGF0 YS5jcHAKaW5kZXggNjRiMTc1NC4uZWIyMzBkNSAxMDA2NDQKLS0tIGEvV2ViS2l0L2Nocm9taXVt L3NyYy9XZWJQYXNzd29yZEZvcm1EYXRhLmNwcAorKysgYi9XZWJLaXQvY2hyb21pdW0vc3JjL1dl YlBhc3N3b3JkRm9ybURhdGEuY3BwCkBAIC0xNjIsNyArMTYyLDEwIEBAIFdlYlBhc3N3b3JkRm9y bURhdGE6OldlYlBhc3N3b3JkRm9ybURhdGEoY29uc3QgV2ViRm9ybUVsZW1lbnQmIHdlYkZvcm0p CiAgICAgS1VSTCBmdWxsT3JpZ2luKFBhcnNlZFVSTFN0cmluZywgZm9ybS0+ZG9jdW1lbnQoKS0+ ZG9jdW1lbnRVUkkoKSk7CiAKICAgICAvLyBDYWxjdWxhdGUgdGhlIGNhbm9uaWNhbCBhY3Rpb24g VVJMCi0gICAgS1VSTCBmdWxsQWN0aW9uID0gZnJhbWUtPmxvYWRlcigpLT5jb21wbGV0ZVVSTChm b3JtLT5hY3Rpb24oKSk7CisgICAgU3RyaW5nIGFjdGlvbiA9IGZvcm0tPmFjdGlvbigpOworICAg IGlmIChhY3Rpb24uaXNOdWxsKCkpCisgICAgICAgIGFjdGlvbiA9ICIiOyAvLyBtaXNzaW5nICdh Y3Rpb24nIGF0dHJpYnV0ZSBpbXBsaWVzIGN1cnJlbnQgVVJMCisgICAgS1VSTCBmdWxsQWN0aW9u ID0gZnJhbWUtPmxvYWRlcigpLT5jb21wbGV0ZVVSTChhY3Rpb24pOwogICAgIGlmICghZnVsbEFj dGlvbi5pc1ZhbGlkKCkpCiAgICAgICAgIHJldHVybjsKIAo=