| Summary: | REGRESSION(r143076): Crash when calling removeNamedItem or removeNamedItemNS with a non-existent attribute of newly created element | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Peter Nelson <peter> | ||||||
| Component: | DOM | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bfulgham, cmarcelo, kling, ojan.autocc, peter, webkit.review.bot | ||||||
| Priority: | P1 | Keywords: | Regression | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
Created attachment 188720 [details]
Patch
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?
(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. 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.
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!
(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? (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. Committed r143115: <http://trac.webkit.org/changeset/143115> |
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.