[webkit-reviews] review denied: [Bug 33983] Setting <Select>.options.length to remove elements is broken if the page has (or ever had) DOM mutation events registered : [Attachment 47260] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied James Robinson <jamesr at chromium.org>'s
request for review:
Bug 33983: Setting <Select>.options.length to remove elements is broken if the
page has (or ever had) DOM mutation events registered
https://bugs.webkit.org/show_bug.cgi?id=33983

Attachment 47260: Patch
https://bugs.webkit.org/attachment.cgi?id=47260&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> -	   const Vector<Element*>& items = listItems();
> +	   const Vector<Element*> items = listItems();

The const here should also be removed.

If mutation events are involved, then it's not enough to copy the vector. We
also have to ref() the element pointers because there's no guarantee the
mutation event handlers will not eliminate the last reference to one of the
list items.

So this should be a Vector<RefPtr<Element> > and the code should be written to
put the items into that vector.

I'm going to say review- because it seems that if we're going to change this we
should address that problem as well as this more obvious one.