[webkit-reviews] review granted: [Bug 13894] Remove the legacy class KJS::JSHTMLElement : [Attachment 14752] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 13894: Remove the legacy class KJS::JSHTMLElement
http://bugs.webkit.org/show_bug.cgi?id=13894

Attachment 14752: patch
http://bugs.webkit.org/attachment.cgi?id=14752&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+    if (impl()->hasTagName(aTag))
+	 return UString(static_cast<const HTMLAnchorElement*>(impl())->href());


Shouldn't this code be in JSHTMLAnchorElement instead?

+static HTMLFormElement* getForm(HTMLElement* element)
+{
+    if (element->isGenericFormElement())
+	 return static_cast<HTMLGenericFormElement*>(element)->form();
+    if (element->hasTagName(labelTag))
+	 return static_cast<HTMLLabelElement*>(element)->form();
+    if (element->hasTagName(objectTag))
+	 return static_cast<HTMLObjectElement*>(element)->form();
+
+    return 0;
+}

This should instead of a virtual function; maybe named
formForEventHandlerScope. The base implementation could be in HTMLElement (with
most of the form code from JSHTMLElement::pushEventHandlerScope), and then the
overriden versions for subclasses to do what getForm does.

+HTMLElement* toHTMLElement(JSValue *val)
+{
+    if (!val || !val->isObject(&JSHTMLElement::info))
+	 return 0;
+    return
static_cast<HTMLElement*>(static_cast<JSHTMLElement*>(val)->impl());
+}

Don't functions like that get auto-generated for us? Why do we have to write
that one? I don't think it belongs in kjs_html.h/cpp regardless.

+// Runtime object support code for JSHTMLAppletElement, JSHTMLEmbedElement and
JSHTMLObjectElement.

Seems like this code needs to move into a new source file.

r=me