Bug 25741

Summary:

insertItemBefore boundary condition incorrect

Product:

WebKit

Reporter:

Adam Barth <abarth>

Component:

SVG

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, oliver

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
patch (no tests)	none
patch	none
better patch	oliver: review+
even better patch?	oliver: review+

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	Details Formatted Diff Diff
patch (7.87 KB, patch) 2009-05-15 17:21 PDT, Adam Barth	no flags	Details Formatted Diff Diff
better patch (7.77 KB, patch) 2009-05-15 17:27 PDT, Adam Barth	oliver: review+	Details Formatted Diff Diff
even better patch? (8.62 KB, patch) 2009-05-15 17:52 PDT, Adam Barth	oliver: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

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.