[webkit-reviews] review granted: [Bug 10934] REGRESSION: prototype.js logs error (HTMLFormElement) on webkit builds : [Attachment 12588] proposed fix

[bugzilla-request-daemon at macosforge.org]

Darin Adler <darin at apple.com> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 10934: REGRESSION: prototype.js logs error (HTMLFormElement) on webkit
builds
http://bugs.webkit.org/show_bug.cgi?id=10934

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

------- Additional Comments from Darin Adler <darin at apple.com>
+	 select.remove((static_cast<HTMLOptionElement*>(element))->index());

There's an extra set of parentheses on this line that is not needed.

+    if (element && element->hasTagName(optionTag))
+	 select.remove((static_cast<HTMLOptionElement*>(element))->index());
+    else
+	 select.remove(static_cast<int>(args[0]->toNumber(exec)));

I think it would be a bit more elegant to do the HTMLElement part of this by
overloading HTMLSelectElement::remove to have a version that takes an
HTMLElement*. The HTMLSelectElement::add function already takes an HTMLElement,
so I think it would be consistent.

Then we could also fix the bug where you can call remove on select A with an
HTMLOptionElement from select B and it will remove one of the options from
select A!

I think that eventually we should fix -[HTMLSelectElement length], even though
it's good to preserve the strange version of it for this patch.

Fine to land as-is, r=me.