97898 2012-09-28 05:20:40 -0700 [Microdata] names.item() must return null for out of range indexes 2012-10-01 03:45:16 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 92986 1 arko arko abarth haraken ojan rniwa webkit.review.bot oldest_to_newest 730532 0 arko 2012-09-28 05:20:40 -0700 DOMStringList.item() must return null for an invalid index. Sample Test: test(function () { var testEl = makeEl('div',{}); assert_equals( testEl.properties.names.item(0), null, 'item(0)' ); assert_equals( testEl.properties.names.item(-1), null, 'item(-1)' ); }, 'names.item() must return null for out of range indexes'); Expected: testEl.properties.names.item(-1) should return null Actual: INDEX_SIZE_ERR: DOM Exception 1: Index or size was negative, or greater than the allowed value According to the spec http://www.w3.org/TR/DOM-Level-3-Core/core.html#DOMStringList item() must return DOMString, The DOMString at the indexth position in the DOMStringList, or null if that is not a valid index. Currently it throws "INDEX_SIZE_ERR: DOM Exception 1: Index or size was negative, or greater than the allowed value", when index is negative. 730576 1 166236 arko 2012-09-28 06:11:52 -0700 Created attachment 166236 Patch 730658 2 166236 haraken 2012-09-28 08:01:23 -0700 Comment on attachment 166236 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166236&action=review The implementation looks good. I am just concerned about cross-browser compatibility. > Source/WebCore/ChangeLog:8 > + DOMStringList.item() must return null for an invalid index. What is the behavior of other browsers (IE, Firefox, Opera)? 730660 3 arko 2012-09-28 08:05:58 -0700 (In reply to comment #2) > (From update of attachment 166236 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166236&action=review > > The implementation looks good. I am just concerned about cross-browser compatibility. > > > Source/WebCore/ChangeLog:8 > > + DOMStringList.item() must return null for an invalid index. > > What is the behavior of other browsers (IE, Firefox, Opera)? I tested in Opera and Firefox. Both return null in case of negative index. Unable to test it in as I don't have access to IE. 730662 4 166236 haraken 2012-09-28 08:08:02 -0700 Comment on attachment 166236 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166236&action=review >>> Source/WebCore/ChangeLog:8 >>> + DOMStringList.item() must return null for an invalid index. >> >> What is the behavior of other browsers (IE, Firefox, Opera)? > > I tested in Opera and Firefox. Both return null in case of negative index. Unable to test it in as I don't have access to IE. Thanks for the clarification. Given that the spec also requires it, the change looks reasonable. 730665 5 166236 haraken 2012-09-28 08:11:45 -0700 Comment on attachment 166236 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166236&action=review >>>> Source/WebCore/ChangeLog:8 >>>> + DOMStringList.item() must return null for an invalid index. >>> >>> What is the behavior of other browsers (IE, Firefox, Opera)? >> >> I tested in Opera and Firefox. Both return null in case of negative index. Unable to test it in as I don't have access to IE. > > Thanks for the clarification. Given that the spec also requires it, the change looks reasonable. It would be better to describe other browsers' behaviors in the ChangeLog. 730675 6 arko 2012-09-28 08:26:41 -0700 (In reply to comment #5) > (From update of attachment 166236 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166236&action=review > > >>>> Source/WebCore/ChangeLog:8 > >>>> + DOMStringList.item() must return null for an invalid index. > >>> > >>> What is the behavior of other browsers (IE, Firefox, Opera)? > >> > >> I tested in Opera and Firefox. Both return null in case of negative index. Unable to test it in as I don't have access to IE. > > > > Thanks for the clarification. Given that the spec also requires it, the change looks reasonable. > > It would be better to describe other browsers' behaviors in the ChangeLog. Sure I will update the Change log with other browsers behavior. Thanks for the review. 731541 7 arko 2012-10-01 03:45:16 -0700 Committed r130026: <http://trac.webkit.org/changeset/130026> 166236 2012-09-28 06:11:52 -0700 2012-09-28 08:11:45 -0700 Patch bug-97898-20120928183817.patch text/plain 4320 arko SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEyOTg4MSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA5LTI4ICBBcmtvIFNh aGEgIDxhcmtvQG1vdG9yb2xhLmNvbT4KKworICAgICAgICBNaWNyb2RhdGE6IG5hbWVzLml0ZW0o KSBtdXN0IHJldHVybiBudWxsIGZvciBvdXQgb2YgcmFuZ2UgaW5kZXhlcy4KKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTk3ODk4CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRE9NU3RyaW5nTGlzdC5pdGVtKCkg bXVzdCByZXR1cm4gbnVsbCBmb3IgYW4gaW52YWxpZCBpbmRleC4KKyAgICAgICAgU3BlYzogaHR0 cDovL3d3dy53My5vcmcvVFIvRE9NLUxldmVsLTMtQ29yZS9jb3JlLmh0bWwjRE9NU3RyaW5nTGlz dAorICAgICAgICBSZW1vdmVkIFtJc0luZGV4XSBleHRlbmRlZCBJREwgYXR0cmlidXRlIGZyb20g aXRlbSgpIG1ldGhvZCdzIGluZGV4CisgICAgICAgIHBhcmFtZXRlciBpbiBET01TdHJpbmdMaXN0 IGludGVyZmFjZS4KKworICAgICAgICBUZXN0OiBmYXN0L2RvbS9NaWNyb0RhdGEvbmFtZXMtaXRl bS1vdXQtb2YtcmFuZ2UtaW5kZXguaHRtbAorCisgICAgICAgICogZG9tL0RPTVN0cmluZ0xpc3Qu aWRsOgorCiAyMDEyLTA5LTI4ICBTdWRhcnNhbmEgTmFnaW5lbmkgIDxzdWRhcnNhbmEubmFnaW5l bmlAaW50ZWwuY29tPgogCiAgICAgICAgIGVkaXRpbmcvcGFzdGVib2FyZC9wYXN0ZS1yZW1vdmlu Zy1pZnJhbWUuaHRtbCBjcmFzaGVzIG9uIEVGTCBib3RzCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9k b20vRE9NU3RyaW5nTGlzdC5pZGwKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvZG9tL0RP TVN0cmluZ0xpc3QuaWRsCShyZXZpc2lvbiAxMjk4NzcpCisrKyBTb3VyY2UvV2ViQ29yZS9kb20v RE9NU3RyaW5nTGlzdC5pZGwJKHdvcmtpbmcgY29weSkKQEAgLTMwLDcgKzMwLDcgQEAgbW9kdWxl IGNvcmUgewogICAgICAgICBKU0N1c3RvbVRvTmF0aXZlT2JqZWN0CiAgICAgXSBET01TdHJpbmdM aXN0IHsKICAgICAgICAgcmVhZG9ubHkgYXR0cmlidXRlIHVuc2lnbmVkIGxvbmcgbGVuZ3RoOwot ICAgICAgICBbVHJlYXRSZXR1cm5lZE51bGxTdHJpbmdBcz1OdWxsXSBET01TdHJpbmcgaXRlbShp biBbSXNJbmRleCxPcHRpb25hbD1EZWZhdWx0SXNVbmRlZmluZWRdIHVuc2lnbmVkIGxvbmcgaW5k ZXgpOworICAgICAgICBbVHJlYXRSZXR1cm5lZE51bGxTdHJpbmdBcz1OdWxsXSBET01TdHJpbmcg aXRlbShpbiBbT3B0aW9uYWw9RGVmYXVsdElzVW5kZWZpbmVkXSB1bnNpZ25lZCBsb25nIGluZGV4 KTsKICAgICAgICAgYm9vbGVhbiBjb250YWlucyhpbiBbT3B0aW9uYWw9RGVmYXVsdElzVW5kZWZp bmVkXSBET01TdHJpbmcgc3RyaW5nKTsKICAgICB9OwogCkluZGV4OiBMYXlvdXRUZXN0cy9DaGFu Z2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCShyZXZpc2lvbiAxMjk4 ODEpCisrKyBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwx NSBAQAorMjAxMi0wOS0yOCAgQXJrbyBTYWhhICA8YXJrb0Btb3Rvcm9sYS5jb20+CisKKyAgICAg ICAgTWljcm9kYXRhOiBuYW1lcy5pdGVtKCkgbXVzdCByZXR1cm4gbnVsbCBmb3Igb3V0IG9mIHJh bmdlIGluZGV4ZXMuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD05Nzg5OAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIEFkZGVkIHRlc3QgY2FzZSB0byBlbnN1cmUgbmFtZXMuaXRlbSgpIGJlaGF2aW9yIGZvciBv dXQgb2YgcmFuZ2UgaW5kZXhlcy4KKworICAgICAgICAqIGZhc3QvZG9tL01pY3JvRGF0YS9uYW1l cy1pdGVtLW91dC1vZi1yYW5nZS1pbmRleC1leHBlY3RlZC50eHQ6IEFkZGVkLgorICAgICAgICAq IGZhc3QvZG9tL01pY3JvRGF0YS9uYW1lcy1pdGVtLW91dC1vZi1yYW5nZS1pbmRleC5odG1sOiBB ZGRlZC4KKwogMjAxMi0wOS0yOCAgU3VkYXJzYW5hIE5hZ2luZW5pICA8c3VkYXJzYW5hLm5hZ2lu ZW5pQGludGVsLmNvbT4KIAogICAgICAgICBlZGl0aW5nL3Bhc3RlYm9hcmQvcGFzdGUtcmVtb3Zp bmctaWZyYW1lLmh0bWwgY3Jhc2hlcyBvbiBFRkwgYm90cwpJbmRleDogTGF5b3V0VGVzdHMvZmFz dC9kb20vTWljcm9EYXRhL25hbWVzLWl0ZW0tb3V0LW9mLXJhbmdlLWluZGV4LWV4cGVjdGVkLnR4 dAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9mYXN0L2RvbS9NaWNyb0RhdGEvbmFtZXMtaXRl bS1vdXQtb2YtcmFuZ2UtaW5kZXgtZXhwZWN0ZWQudHh0CShyZXZpc2lvbiAwKQorKysgTGF5b3V0 VGVzdHMvZmFzdC9kb20vTWljcm9EYXRhL25hbWVzLWl0ZW0tb3V0LW9mLXJhbmdlLWluZGV4LWV4 cGVjdGVkLnR4dAkocmV2aXNpb24gMCkKQEAgLTAsMCArMSwxNSBAQAorVGhpcyB0ZXN0IHZlcmlm aWVzIHByb3BlcnRpZXMubmFtZXMuaXRlbShpbmRleCkgYmVoYXZpb3IgZm9yIG91dC1vZi1yYW5n ZSBpbmRleC4KKworT24gc3VjY2VzcywgeW91IHdpbGwgc2VlIGEgc2VyaWVzIG9mICJQQVNTIiBt ZXNzYWdlcywgZm9sbG93ZWQgYnkgIlRFU1QgQ09NUExFVEUiLgorCisKK1Rlc3QgaXRlbVR5cGVb aW5kZXhdCitQQVNTIGVsZW1lbnQucHJvcGVydGllcy5uYW1lcy5pdGVtKDApIGlzIG51bGwKK1BB U1MgZWxlbWVudC5wcm9wZXJ0aWVzLm5hbWVzLml0ZW0oLTEpIGlzIG51bGwKK1BBU1MgZWxlbWVu dC5wcm9wZXJ0aWVzLm5hbWVzLml0ZW0oMCkgaXMgJ2ZvbycKK1BBU1MgZWxlbWVudC5wcm9wZXJ0 aWVzLm5hbWVzLml0ZW0oMSkgaXMgbnVsbAorUEFTUyBlbGVtZW50LnByb3BlcnRpZXMubmFtZXMu aXRlbSgtMSkgaXMgbnVsbAorUEFTUyBzdWNjZXNzZnVsbHlQYXJzZWQgaXMgdHJ1ZQorCitURVNU IENPTVBMRVRFCisKSW5kZXg6IExheW91dFRlc3RzL2Zhc3QvZG9tL01pY3JvRGF0YS9uYW1lcy1p dGVtLW91dC1vZi1yYW5nZS1pbmRleC5odG1sCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL2Zh c3QvZG9tL01pY3JvRGF0YS9uYW1lcy1pdGVtLW91dC1vZi1yYW5nZS1pbmRleC5odG1sCShyZXZp c2lvbiAwKQorKysgTGF5b3V0VGVzdHMvZmFzdC9kb20vTWljcm9EYXRhL25hbWVzLWl0ZW0tb3V0 LW9mLXJhbmdlLWluZGV4Lmh0bWwJKHJldmlzaW9uIDApCkBAIC0wLDAgKzEsMjIgQEAKKzwhRE9D VFlQRSBodG1sPgorPGh0bWw+Cis8aGVhZD4KKzxzY3JpcHQgc3JjPSIuLi8uLi9qcy9yZXNvdXJj ZXMvanMtdGVzdC1wcmUuanMiPjwvc2NyaXB0PgorPC9oZWFkPgorPGJvZHk+Cis8ZGl2IGl0ZW1z Y29wZSBpZD0ibXlkaXYiPjwvZGl2PgorPHNjcmlwdD4KK2Rlc2NyaXB0aW9uKCJUaGlzIHRlc3Qg dmVyaWZpZXMgcHJvcGVydGllcy5uYW1lcy5pdGVtKGluZGV4KSBiZWhhdmlvciBmb3Igb3V0LW9m LXJhbmdlIGluZGV4LiIpOworCitkZWJ1ZygiVGVzdCBpdGVtVHlwZVtpbmRleF0iKTsKK3ZhciBl bGVtZW50ID0gZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoIm15ZGl2Iik7CitzaG91bGRCZU51bGwo ImVsZW1lbnQucHJvcGVydGllcy5uYW1lcy5pdGVtKDApIik7CitzaG91bGRCZU51bGwoImVsZW1l bnQucHJvcGVydGllcy5uYW1lcy5pdGVtKC0xKSIpOworZWxlbWVudC5pbm5lckhUTUwgPSAiPGRp diBpdGVtcHJvcD0nZm9vJz48L2RpdiI7CitzaG91bGRCZSgiZWxlbWVudC5wcm9wZXJ0aWVzLm5h bWVzLml0ZW0oMCkiLCAiJ2ZvbyciKTsKK3Nob3VsZEJlTnVsbCgiZWxlbWVudC5wcm9wZXJ0aWVz Lm5hbWVzLml0ZW0oMSkiKTsKK3Nob3VsZEJlTnVsbCgiZWxlbWVudC5wcm9wZXJ0aWVzLm5hbWVz Lml0ZW0oLTEpIik7Cis8L3NjcmlwdD4KKzxzY3JpcHQgc3JjPSIuLi8uLi9qcy9yZXNvdXJjZXMv anMtdGVzdC1wb3N0LmpzIj48L3NjcmlwdD4KKzwvYm9keT4KKzwvaHRtbD4K