Bug 13894 – Remove the legacy class KJS::JSHTMLElement

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Sam Weinig]

Now that all the children are autogenerated, we can finally get rid of KJS::JSHTMLElement!!! and have a more proper prototype chain.  Patch forthcoming.

[Sam Weinig]

Created attachment 14752 [details]
patch

[Darin Adler]

Comment on attachment 14752 [details]
patch

+    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

[Sam Weinig]

(In reply to comment #2)
> (From update of attachment 14752 [details] [edit])
> +    if (impl()->hasTagName(aTag))
> +        return UString(static_cast<const HTMLAnchorElement*>(impl())->href());

Made this change.

> +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.

I made formForEventHandlerScope in HTMLElement but kept the logic of JSHTMLElement::pushEventHandlerScope in JSHTMLElement for now because it seems like the correct place for it.

> +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.

Done.

> +// Runtime object support code for JSHTMLAppletElement, JSHTMLEmbedElement and
> JSHTMLObjectElement.
> 
> Seems like this code needs to move into a new source file.

I agree, I will move it in another patch though, as this patch isn't really related.

[Sam Weinig]

Landed in r21842.