Bug 13894

Created attachment 14752 [details] patch

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

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

Landed in r21842.