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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 26 16:23:47 PST 2010


James Robinson <jamesr at chromium.org> has asked	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 47460: patch
https://bugs.webkit.org/attachment.cgi?id=47460&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
This patch uses the Vector<RefPtr<..> > approach and adds a bunch more tests. 
The algorithm is very simple, when the 'length' is set to something shorter
than the current length, the code first builds a list of all option elements
past the new 'length' and adds them to a list to be removed.  It then iterates
through the list and attempts to call removeChild() on each of them, assuming
that they still have a parent.	The following actions are all possible during
the second iteration:

1.) A mutation event handler removes an element that is already scheduled to be
deleted (covered by select-set-length-with-mutation-remove.html).  In this case
the remove just doesn't happen.
2.) A mutation event handler reorders options after the truncation has begun by
causing an option that was scheduled to be removed to be earlier in the list
(covered by select-set-length-with-mutation-reorder.html).  In this case the
items scheduled to be removed are still removed regardless of their new
position.
3.) A mutation event handler moves an option that is scheduled to be removed to
be a child of a different select element (covered by
select-set-length-with-mutation-reparent.html).  In this case the element is
removed from its new parent.

The odd behavior in case 3 matches what Firefox 3.5 and IE8 do.  WebKit ToT
crashes on some of these tests without this patch applied.  With this patch, we
match Firefox 3.5 and IE8 on all cases.


More information about the webkit-reviews mailing list