641409 - Move methods using only GetType from nsGenericHTMLFormElement to nsIFormControl

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

That way, we can make those methods inline and non-virtual.

Unfortunately:
- GetType() is still virtual and making it non-virtual might not be doable (does it worth adding a PRUint32 to all form controls?);
- I've been told that I still have to use PRBool in interfaces. That makes me really sad :(

A possible follow-up would be to create a structure that keeps track of form control properties and make those methods more maintainable.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I thought we were moving things the other way? Will this patch break anything for elements that don't have a frame?

Comment 2

[Mounir Lamouri (:mounir)]

(In reply to comment #1)
> I thought we were moving things the other way?

What do you mean? The idea here is that we declare a method in nsIFormControl and define it in nsGenericHTMLFormElement, thus making the method virtual. But we could have the definition in the interface quite easily.

> Will this patch break anything for elements that don't have a frame?

nsGenericHTMLFormElement inherits from nsIFormControl so it shouldn't change anything.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> - I've been told that I still have to use PRBool in interfaces.

Hmm.  Only ones we expect to be called from outside our code, I'd think.

Mounir, if we really cared we might be able to stuff the type into element-specific flags once we make flags 64-bit on nodes, right?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 519064 [details] [diff] [review]
Patch v1

The two-argument form of IsSingleLineTextControl should probably be protected.

r=me with that.

Comment 5

[Mounir Lamouri (:mounir)]

I've changed:
public PRBool IsSingleLineTextControl(PRBool, PRUint32)
to:
protected bool IsSingleLineTextControl(bool, PRUint32)

Thanks for the quick review.

Comment 6

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/364e0b9a2168