[webkit-reviews] review denied: [Bug 13287] Cannot change SELECT to a dynamically created option : [Attachment 21176] proposed fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat May 24 22:41:05 PDT 2008


Darin Adler <darin at apple.com> has denied Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 13287: Cannot change SELECT to a dynamically created option
http://bugs.webkit.org/show_bug.cgi?id=13287

Attachment 21176: proposed fix
http://bugs.webkit.org/attachment.cgi?id=21176&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+	 if (newChildPtr->isElementNode() &&
static_cast<Element*>(newChildPtr)->hasLocalName(optionTag) && !m_multiple) {

As far as I can tell, nothing guarantees that the node pointed to by newChild
won't be deallocated by this point. You will need to use a RefPtr here. Maybe
the right thing to do is to use the "prp" pattern (naming the argument
prpNewChild and the local RefPtr newChild).

Ideally we'd make a test case that uses DOM mutation events to make this
unlikely even occur.

Isn't there a better hook we can use to deal with the change in children? At
the very least I'd like to see the repeated code shared in a single function
rather than pasted three times.

Patch otherwise looks good.


More information about the webkit-reviews mailing list