[webkit-reviews] review denied: [Bug 5784] Assertion failure in DocumentImpl::removeElementById() (idCount > 0) : [Attachment 4757] Call updateId() when inserting id attribute

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied opendarwin.org at mitzpettel.com's
request for review:
Bug 5784: Assertion failure in DocumentImpl::removeElementById() (idCount > 0)
http://bugzilla.opendarwin.org/show_bug.cgi?id=5784

Attachment 4757: Call updateId() when inserting id attribute
http://bugzilla.opendarwin.org/attachment.cgi?id=4757&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
The change here is just right to fix the bug, but I'm slightly concerned about
the effect this might have on performance. I'm also annoyed about that
dom_elementimpl.h now includes the htmlnames.h header.

Perhaps a better change would be to use a different function in htmlparser.cpp
and leave this one as a simpler and faster function only to be used by
htmltokenizer.cpp. We could add an assertion that element is 0.

There's a chance that we could just use setAttribute in the two code paths
inside htmlparser.cpp, and in fact I think that skipping incDOMTreeVersion and
attributeChanged might be bad for those cases.

Lets look into a patch that involves making insertAttribute assert more and get
called less.