90111 2012-06-27 16:42:55 -0700 Cleanup HTMLFormCollection 2012-06-28 16:03:09 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 90118 1 rniwa rniwa darin kling koivisto menard tkent oldest_to_newest 658964 0 rniwa 2012-06-27 16:42:55 -0700 As the comment at the beginning of HTMLFormCollection.h this entire class is a giant hack, and indeed the class is filled with questionable code. Clean up some of that mess first so that we can do more refactoring later. 658982 1 149824 rniwa 2012-06-27 16:57:43 -0700 Created attachment 149824 Cleanup 658985 2 149824 rniwa 2012-06-27 17:00:06 -0700 Comment on attachment 149824 Cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=149824&action=review > Source/WebCore/html/HTMLFormCollection.cpp:-176 > - m_cache.current = getNamedItem(idAttr, name); > - if (m_cache.current) > - return m_cache.current; > - m_cache.current = getNamedItem(nameAttr, name); > - return m_cache.current; Updating m_cache.current here made no sense. In fact, I bet you can find a test case where this introduces a bug using the Objective-C binding. e.g. collection.item(0); collection.namedItem('blah'); collection.item(0); The second call to item() returns the element with id/name 'blah'. Unfortunately, ES5 binding uses namedItems so we can't add a layout test for it. 659671 3 149824 kling 2012-06-28 12:00:03 -0700 Comment on attachment 149824 Cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=149824&action=review > Source/WebCore/ChangeLog:8 > + Got rid of getNamedItem and enamed getNamedFormItem to firstNamedItem and got rid of duplicateNumber argument since Typo, enamed. > Source/WebCore/html/HTMLFormCollection.cpp:126 > + if (elementsArray[i]->isEnumeratable() && element->fastGetAttribute(attrName) == name) Maybe someone should rename isEnumeratable() this to isEnumerable() someday. 659909 4 rniwa 2012-06-28 16:01:10 -0700 Committed r121478: <http://trac.webkit.org/changeset/121478> 659911 5 rniwa 2012-06-28 16:03:09 -0700 (In reply to comment #4) > Committed r121478: <http://trac.webkit.org/changeset/121478> I've figured out a way to write a WebKit API test so I went ahead and landed it with it. Fixed the typo in http://trac.webkit.org/changeset/121479. 149824 2012-06-27 16:57:43 -0700 2012-06-28 12:00:02 -0700 Cleanup bug-90111-20120627170231.patch text/plain 5388 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEyMTM4MykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDEyLTA2LTI3ICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIENsZWFudXAgSFRNTEZvcm1Db2xs ZWN0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05 MDExMQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEdv dCByaWQgb2YgZ2V0TmFtZWRJdGVtIGFuZCBlbmFtZWQgZ2V0TmFtZWRGb3JtSXRlbSB0byBmaXJz dE5hbWVkSXRlbSBhbmQgZ290IHJpZCBvZiBkdXBsaWNhdGVOdW1iZXIgYXJndW1lbnQgc2luY2UK KyAgICAgICAgaXQncyBhbHdheXMgMC4gQWxzbyBtYWRlIGl0IGEgc3RhdGljIGxvY2FsIGZ1bmN0 aW9uLiBJbiBhZGRpdGlvbiwgcmVtb3ZlZCBuZXh0SXRlbSgpIHNpbmNlIGl0J3Mgbm90IHVzZWQg YW55d2hlcmUuCisKKyAgICAgICAgKiBodG1sL0hUTUxGb3JtQ29sbGVjdGlvbi5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpmaXJzdE5hbWVkSXRlbSk6CisgICAgICAgIChXZWJDb3JlKToKKyAgICAg ICAgKFdlYkNvcmU6OkhUTUxGb3JtQ29sbGVjdGlvbjo6bmFtZWRJdGVtKToKKyAgICAgICAgKiBo dG1sL0hUTUxGb3JtQ29sbGVjdGlvbi5oOgorICAgICAgICAoSFRNTEZvcm1Db2xsZWN0aW9uKToK KwogMjAxMi0wNi0yNSAgTWFyayBIYWhuZW5iZXJnICA8bWhhaG5lbmJlcmdAYXBwbGUuY29tPgog CiAgICAgICAgIEpTTG9jayBzaG91bGQgYmUgcGVyLUpTR2xvYmFsRGF0YQpJbmRleDogU291cmNl L1dlYkNvcmUvaHRtbC9IVE1MRm9ybUNvbGxlY3Rpb24uY3BwCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJj ZS9XZWJDb3JlL2h0bWwvSFRNTEZvcm1Db2xsZWN0aW9uLmNwcAkocmV2aXNpb24gMTIxMzQ3KQor KysgU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MRm9ybUNvbGxlY3Rpb24uY3BwCSh3b3JraW5nIGNv cHkpCkBAIC0xMTYsNTEgKzExNiwyOSBAQCBOb2RlKiBIVE1MRm9ybUNvbGxlY3Rpb246Oml0ZW0o dW5zaWduZWQgCiAgICAgcmV0dXJuIDA7CiB9CiAKLUVsZW1lbnQqIEhUTUxGb3JtQ29sbGVjdGlv bjo6Z2V0TmFtZWRJdGVtKGNvbnN0IFF1YWxpZmllZE5hbWUmIGF0dHJOYW1lLCBjb25zdCBBdG9t aWNTdHJpbmcmIG5hbWUpIGNvbnN0CitzdGF0aWMgSFRNTEVsZW1lbnQqIGZpcnN0TmFtZWRJdGVt KGNvbnN0IFZlY3RvcjxGb3JtQXNzb2NpYXRlZEVsZW1lbnQqPiYgZWxlbWVudHNBcnJheSwKKyAg ICBjb25zdCBWZWN0b3I8SFRNTEltYWdlRWxlbWVudCo+KiBpbWFnZUVsZW1lbnRzQXJyYXksIGNv bnN0IFF1YWxpZmllZE5hbWUmIGF0dHJOYW1lLCBjb25zdCBTdHJpbmcmIG5hbWUpCiB7Ci0gICAg bV9jYWNoZS5wb3NpdGlvbiA9IDA7Ci0gICAgcmV0dXJuIGdldE5hbWVkRm9ybUl0ZW0oYXR0ck5h bWUsIG5hbWUsIDApOwotfQotCi1FbGVtZW50KiBIVE1MRm9ybUNvbGxlY3Rpb246OmdldE5hbWVk Rm9ybUl0ZW0oY29uc3QgUXVhbGlmaWVkTmFtZSYgYXR0ck5hbWUsIGNvbnN0IFN0cmluZyYgbmFt ZSwgaW50IGR1cGxpY2F0ZU51bWJlcikgY29uc3QKLXsKLSAgICBjb25zdCBWZWN0b3I8Rm9ybUFz c29jaWF0ZWRFbGVtZW50Kj4mIGVsZW1lbnRzQXJyYXkgPSBmb3JtQ29udHJvbEVsZW1lbnRzKCk7 CisgICAgQVNTRVJUKGF0dHJOYW1lID09IGlkQXR0ciB8fCBhdHRyTmFtZSA9PSBuYW1lQXR0cik7 CiAKLSAgICBib29sIGZvdW5kSW5wdXRFbGVtZW50cyA9IGZhbHNlOwogICAgIGZvciAodW5zaWdu ZWQgaSA9IDA7IGkgPCBlbGVtZW50c0FycmF5LnNpemUoKTsgKytpKSB7Ci0gICAgICAgIEZvcm1B c3NvY2lhdGVkRWxlbWVudCogYXNzb2NpYXRlZEVsZW1lbnQgPSBlbGVtZW50c0FycmF5W2ldOwot ICAgICAgICBIVE1MRWxlbWVudCogZWxlbWVudCA9IHRvSFRNTEVsZW1lbnQoYXNzb2NpYXRlZEVs ZW1lbnQpOwotICAgICAgICBpZiAoYXNzb2NpYXRlZEVsZW1lbnQtPmlzRW51bWVyYXRhYmxlKCkg JiYgZWxlbWVudC0+Z2V0QXR0cmlidXRlKGF0dHJOYW1lKSA9PSBuYW1lKSB7Ci0gICAgICAgICAg ICBmb3VuZElucHV0RWxlbWVudHMgPSB0cnVlOwotICAgICAgICAgICAgaWYgKCFkdXBsaWNhdGVO dW1iZXIpCi0gICAgICAgICAgICAgICAgcmV0dXJuIGVsZW1lbnQ7Ci0gICAgICAgICAgICAtLWR1 cGxpY2F0ZU51bWJlcjsKLSAgICAgICAgfQorICAgICAgICBIVE1MRWxlbWVudCogZWxlbWVudCA9 IHRvSFRNTEVsZW1lbnQoZWxlbWVudHNBcnJheVtpXSk7CisgICAgICAgIGlmIChlbGVtZW50c0Fy cmF5W2ldLT5pc0VudW1lcmF0YWJsZSgpICYmIGVsZW1lbnQtPmZhc3RHZXRBdHRyaWJ1dGUoYXR0 ck5hbWUpID09IG5hbWUpCisgICAgICAgICAgICByZXR1cm4gZWxlbWVudDsKICAgICB9CiAKLSAg ICBpZiAoYmFzZSgpLT5oYXNUYWdOYW1lKGZpZWxkc2V0VGFnKSkKKyAgICBpZiAoIWltYWdlRWxl bWVudHNBcnJheSkKICAgICAgICAgcmV0dXJuIDA7CiAKLSAgICBjb25zdCBWZWN0b3I8SFRNTElt YWdlRWxlbWVudCo+JiBpbWFnZUVsZW1lbnRzQXJyYXkgPSBmb3JtSW1hZ2VFbGVtZW50cygpOwot ICAgIGlmICghZm91bmRJbnB1dEVsZW1lbnRzKSB7Ci0gICAgICAgIGZvciAodW5zaWduZWQgaSA9 IDA7IGkgPCBpbWFnZUVsZW1lbnRzQXJyYXkuc2l6ZSgpOyArK2kpIHsKLSAgICAgICAgICAgIEhU TUxJbWFnZUVsZW1lbnQqIGVsZW1lbnQgPSBpbWFnZUVsZW1lbnRzQXJyYXlbaV07Ci0gICAgICAg ICAgICBpZiAoZWxlbWVudC0+Z2V0QXR0cmlidXRlKGF0dHJOYW1lKSA9PSBuYW1lKSB7Ci0gICAg ICAgICAgICAgICAgaWYgKCFkdXBsaWNhdGVOdW1iZXIpCi0gICAgICAgICAgICAgICAgICAgIHJl dHVybiBlbGVtZW50OwotICAgICAgICAgICAgICAgIC0tZHVwbGljYXRlTnVtYmVyOwotICAgICAg ICAgICAgfQotICAgICAgICB9CisgICAgZm9yICh1bnNpZ25lZCBpID0gMDsgaSA8IGltYWdlRWxl bWVudHNBcnJheS0+c2l6ZSgpOyArK2kpIHsKKyAgICAgICAgSFRNTEltYWdlRWxlbWVudCogZWxl bWVudCA9ICgqaW1hZ2VFbGVtZW50c0FycmF5KVtpXTsKKyAgICAgICAgaWYgKGVsZW1lbnQtPmZh c3RHZXRBdHRyaWJ1dGUoYXR0ck5hbWUpID09IG5hbWUpCisgICAgICAgICAgICByZXR1cm4gZWxl bWVudDsKICAgICB9CiAKICAgICByZXR1cm4gMDsKIH0KIAotTm9kZSogSFRNTEZvcm1Db2xsZWN0 aW9uOjpuZXh0SXRlbSgpIGNvbnN0Ci17Ci0gICAgcmV0dXJuIGl0ZW0obV9jYWNoZS5wb3NpdGlv biArIDEpOwotfQotCiBOb2RlKiBIVE1MRm9ybUNvbGxlY3Rpb246Om5hbWVkSXRlbShjb25zdCBB dG9taWNTdHJpbmcmIG5hbWUpIGNvbnN0CiB7CiAgICAgLy8gaHR0cDovL21zZG4ubWljcm9zb2Z0 LmNvbS93b3Jrc2hvcC9hdXRob3IvZGh0bWwvcmVmZXJlbmNlL21ldGhvZHMvbmFtZWRpdGVtLmFz cApAQCAtMTY5LDExICsxNDcsMTAgQEAgTm9kZSogSFRNTEZvcm1Db2xsZWN0aW9uOjpuYW1lZEl0 ZW0oY29ucwogICAgIC8vIG9iamVjdCB3aXRoIGEgbWF0Y2hpbmcgbmFtZSBhdHRyaWJ1dGUsIGJ1 dCBvbmx5IG9uIHRob3NlIGVsZW1lbnRzCiAgICAgLy8gdGhhdCBhcmUgYWxsb3dlZCBhIG5hbWUg YXR0cmlidXRlLgogICAgIGludmFsaWRhdGVDYWNoZUlmTmVlZGVkKCk7Ci0gICAgbV9jYWNoZS5j dXJyZW50ID0gZ2V0TmFtZWRJdGVtKGlkQXR0ciwgbmFtZSk7Ci0gICAgaWYgKG1fY2FjaGUuY3Vy cmVudCkKLSAgICAgICAgcmV0dXJuIG1fY2FjaGUuY3VycmVudDsKLSAgICBtX2NhY2hlLmN1cnJl bnQgPSBnZXROYW1lZEl0ZW0obmFtZUF0dHIsIG5hbWUpOwotICAgIHJldHVybiBtX2NhY2hlLmN1 cnJlbnQ7CisgICAgY29uc3QgVmVjdG9yPEhUTUxJbWFnZUVsZW1lbnQqPiogaW1hZ2VzRWxlbWVu dHMgPSBiYXNlKCktPmhhc1RhZ05hbWUoZmllbGRzZXRUYWcpID8gMCA6ICZmb3JtSW1hZ2VFbGVt ZW50cygpOworICAgIGlmIChOb2RlKiBpdGVtID0gZmlyc3ROYW1lZEl0ZW0oZm9ybUNvbnRyb2xF bGVtZW50cygpLCBpbWFnZXNFbGVtZW50cywgaWRBdHRyLCBuYW1lKSkKKyAgICAgICAgcmV0dXJu IGl0ZW07CisgICAgcmV0dXJuIGZpcnN0TmFtZWRJdGVtKGZvcm1Db250cm9sRWxlbWVudHMoKSwg aW1hZ2VzRWxlbWVudHMsIG5hbWVBdHRyLCBuYW1lKTsKIH0KIAogdm9pZCBIVE1MRm9ybUNvbGxl Y3Rpb246OnVwZGF0ZU5hbWVDYWNoZSgpIGNvbnN0CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9odG1s L0hUTUxGb3JtQ29sbGVjdGlvbi5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2h0bWwv SFRNTEZvcm1Db2xsZWN0aW9uLmgJKHJldmlzaW9uIDEyMTM0NykKKysrIFNvdXJjZS9XZWJDb3Jl L2h0bWwvSFRNTEZvcm1Db2xsZWN0aW9uLmgJKHdvcmtpbmcgY29weSkKQEAgLTQxLDggKzQxLDYg QEAgcHVibGljOgogICAgIHZpcnR1YWwgfkhUTUxGb3JtQ29sbGVjdGlvbigpOwogCiAgICAgdmly dHVhbCBOb2RlKiBpdGVtKHVuc2lnbmVkIGluZGV4KSBjb25zdDsKLSAgICB2aXJ0dWFsIE5vZGUq IG5leHRJdGVtKCkgY29uc3Q7Ci0KICAgICB2aXJ0dWFsIE5vZGUqIG5hbWVkSXRlbShjb25zdCBB dG9taWNTdHJpbmcmIG5hbWUpIGNvbnN0OwogCiBwcml2YXRlOgpAQCAtNTEsOSArNDksNiBAQCBw cml2YXRlOgogICAgIHZpcnR1YWwgdm9pZCB1cGRhdGVOYW1lQ2FjaGUoKSBjb25zdDsKICAgICB2 aXJ0dWFsIHVuc2lnbmVkIGNhbGNMZW5ndGgoKSBjb25zdDsKIAotICAgIEVsZW1lbnQqIGdldE5h bWVkSXRlbShjb25zdCBRdWFsaWZpZWROYW1lJiBhdHRyTmFtZSwgY29uc3QgQXRvbWljU3RyaW5n JiBuYW1lKSBjb25zdDsKLSAgICBFbGVtZW50KiBnZXROYW1lZEZvcm1JdGVtKGNvbnN0IFF1YWxp ZmllZE5hbWUmIGF0dHJOYW1lLCBjb25zdCBTdHJpbmcmIG5hbWUsIGludCBkdXBsaWNhdGVOdW1i ZXIpIGNvbnN0OwotCiAgICAgY29uc3QgVmVjdG9yPEZvcm1Bc3NvY2lhdGVkRWxlbWVudCo+JiBm b3JtQ29udHJvbEVsZW1lbnRzKCkgY29uc3Q7CiAgICAgY29uc3QgVmVjdG9yPEhUTUxJbWFnZUVs ZW1lbnQqPiYgZm9ybUltYWdlRWxlbWVudHMoKSBjb25zdDsKICAgICB1bnNpZ25lZCBudW1iZXJP ZkZvcm1Db250cm9sRWxlbWVudHMoKSBjb25zdDsK