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

[bugzilla-daemon at webkit.org]

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

Attachment 28692: proposed fix
https://bugs.webkit.org/attachment.cgi?id=28692&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   (WebCore::dispatchChildInsertionEvents): Increment DOM tree version.
This will happen when
> +	   dispatching DOMSubtreeModified again, but the version should be
incremented for event
> +	   listeners to have an up to date view of the DOM.
> +	   (WebCore::dispatchChildRemovalEvents): Ditto.

Seems too bad that we later increment the DOM tree version a second time
unnecessarily. But no big problem.

> +	   (WebCore::HTMLSelectElement::add): This function doesn't look at
exception code, so it
> +	   doesn't need to reset it.

This sort of change can be risky; the function can accidentally depend on the
passed-in exception code being zero. Accordingly, I wish you'd make this change
that's not necessarily tied to the rest of the patch in a separate check-in to
possibly help people later who might be tracking down a regression.

r=me