RESOLVED FIXED 25741
insertItemBefore boundary condition incorrect
https://bugs.webkit.org/show_bug.cgi?id=25741
Summary insertItemBefore boundary condition incorrect
Adam Barth
Reported 2009-05-12 16:17:56 PDT
When we get an out-of-bound index here: http://trac.webkit.org/browser/trunk/WebCore/svg/SVGList.h#L99 we should append instead of throwing an error according to the spec: http://www.w3.org/TR/SVG/types.html
Attachments
patch (no tests) (800 bytes, patch)
2009-05-12 23:36 PDT, Adam Barth
no flags
patch (7.87 KB, patch)
2009-05-15 17:21 PDT, Adam Barth
no flags
better patch (7.77 KB, patch)
2009-05-15 17:27 PDT, Adam Barth
oliver: review+
even better patch? (8.62 KB, patch)
2009-05-15 17:52 PDT, Adam Barth
oliver: review+
Adam Barth
Comment 1 2009-05-12 16:18:15 PDT
I have a patch that I can upload when I get home.
Adam Barth
Comment 2 2009-05-12 23:36:24 PDT
Created attachment 30262 [details] patch (no tests) Turns out unhorking my build was too much for Mac Mini to handle tonight. Here's the code part of the patch. I still need to update the tests.
Adam Barth
Comment 3 2009-05-15 17:21:00 PDT
Created attachment 30405 [details] patch patch!
Adam Barth
Comment 4 2009-05-15 17:22:07 PDT
Comment on attachment 30405 [details] patch No, that't not right.
Adam Barth
Comment 5 2009-05-15 17:27:16 PDT
Created attachment 30406 [details] better patch That's better.
Oliver Hunt
Comment 6 2009-05-15 17:35:48 PDT
Comment on attachment 30406 [details] better patch r=me, but i'd prefer it if the test were updated to actually confirm the correct insertion order
Adam Barth
Comment 7 2009-05-15 17:38:47 PDT
(In reply to comment #6) > (From update of attachment 30406 [details] [review]) > r=me, but i'd prefer it if the test were updated to actually confirm the > correct insertion order You mean add the shouldBes after each modification?
Oliver Hunt
Comment 8 2009-05-15 17:42:50 PDT
(In reply to comment #7) > (In reply to comment #6) > > (From update of attachment 30406 [details] [review] [review]) > > r=me, but i'd prefer it if the test were updated to actually confirm the > > correct insertion order > > You mean add the shouldBes after each modification? > Yeah -- i find myself wondering what inserting null is meant to do? (and we;ll probably want to insert distinguishable values)
Adam Barth
Comment 9 2009-05-15 17:52:28 PDT
Created attachment 30407 [details] even better patch? Like the new svglist-insertBeforeItem-appends test?
Oliver Hunt
Comment 10 2009-05-15 17:59:15 PDT
Comment on attachment 30407 [details] even better patch? r=me
Adam Barth
Comment 11 2009-05-15 18:08:11 PDT
Sending LayoutTests/ChangeLog Sending LayoutTests/svg/dom/svglist-exception-on-out-bounds-error-expected.txt Sending LayoutTests/svg/dom/svglist-exception-on-out-bounds-error.html Adding LayoutTests/svg/dom/svglist-insertItemBefore-appends-expected.txt Adding LayoutTests/svg/dom/svglist-insertItemBefore-appends.html Sending WebCore/ChangeLog Sending WebCore/svg/SVGList.h Transmitting file data ....... Committed revision 43795.
Note You need to log in before you can comment on or make changes to this bug.