Bug 110019 – REGRESSION(r143076): Crash when calling removeNamedItem or removeNamedItemNS with a non-existent attribute of newly created element

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Peter Nelson]

Created attachment 188713 [details]
HTML document (based on the Acid3 test) that demonstrates the issue

Currently WebCore crashes when either removeNamedItem or removeNamedItemNS are called with a non-existent attribute of a newly created element.

This bug causes a crash in test 67 of Acid3.

Attached is a small HTML document (based on the Acid3 test) that demonstrates the issue.

Tested with latest nightly on both Windows and OSX.

[Peter Nelson]

Created attachment 188720 [details]
Patch

[Brent Fulgham]

Comment on attachment 188720 [details]
Patch

This seems like a reasonable check. I'm surprised it wasn't needed previously. Is it possible that some external logic is supposed to be preventing a call into this routine if the mode has no attributed?

[Peter Nelson]

(In reply to comment #2)
It may have been changeset 143076 (https://trac.webkit.org/changeset/143076) that broke it -- ElementData is no longer implicitly created when .attributes is referenced.

[Andreas Kling]

Comment on attachment 188720 [details]
Patch

Good catch! Looks like our NamedNodeMap test coverage isn't what it should be. :/
Let's improve that by adding a layout test with this patch.

[Andreas Kling]

Comment on attachment 188720 [details]
Patch

I didn't realize we already have a copy of ACID3 in-tree that catches these bugs.
r=me as we don't need a new test and this looks good. Thanks!

[Brent Fulgham]

(In reply to comment #5)
> (From update of attachment 188720 [details])
> I didn't realize we already have a copy of ACID3 in-tree that catches these bugs.
> r=me as we don't need a new test and this looks good. Thanks!

If we have a test in the tree that covers this logic, why didn't we see it before Peter did? Could we indicate which test covers this behavior in the ChangeLog?

[Andreas Kling]

(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 188720 [details] [details])
> > I didn't realize we already have a copy of ACID3 in-tree that catches these bugs.
> > r=me as we don't need a new test and this looks good. Thanks!
> 
> If we have a test in the tree that covers this logic, why didn't we see it before Peter did? Could we indicate which test covers this behavior in the ChangeLog?

http/tests/misc/acid3.html is crashing on the bots ATM. I'll land this patch and add a comment to the ChangeLog about it.

[Andreas Kling]

Committed r143115: <http://trac.webkit.org/changeset/143115>