345681 - Delete vestigial nsICheckboxControlFrame and nsIRadioControlFrame interface classes

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[jpl24]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060710 Firefox/2.0b1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060710 Firefox/2.0b1

I think this interface can go. Patch coming.

Reproducible: Always

Comment 1

[jpl24]

Attachment: patch_v1

The only potentially dicey part is the static cast in nsHTMLInputElement. I think if the type is NS_FORM_INPUT_CHECKBOX the frame must be a nsGfxCheckboxFrame.

I tested with XBL form widgets turned on and this patch didn't seem to make anything worse.

Comment 2

[jpl24]

Thanks Roc! I did notice that bz was making changes to this file on the reflow branch, so we should coordinate with him.

Comment 3

[Zack Weinberg (:zwol)]

Hm, we have a reviewed patch that never landed, here.  I might try to dust it off; reassigning to myself.

Comment 4

[Zack Weinberg (:zwol)]

Attachment: revised patch

The code is sufficiently different now that I think this needs re-review.  SetCheckboxFaceStyleContext() no longer exists, and the OnChecked method is not doing anything that nsHTMLInputElement couldn't request directly.  Also, I zapped nsIRadioControlFrame while I was in there; it is identical to nsICheckboxControlFrame but for the name.

There's a case for an alternative approach where we preserve OnChecked as a nsFormControlFrame method (or maybe even an nsFrame method) whose default definition does nothing, but nsRadioControlFrame and nsCheckboxControlFrame override it to call InvalidateOverflowRect.  That would have more overhead, but nsHTMLInputElement wouldn't have to know which frames need the invalidate.

Comment 5

[Zack Weinberg (:zwol)]

Attachment: actually revised patch

Thanks for the quick review!  Unfortunately, I forgot to qrefresh, so the patch I attached last night was the same as the old patch.  Doh.  This is the real new patch.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

 NS_QUERYFRAME_HEAD(nsGfxCheckboxControlFrame)
-  NS_QUERYFRAME_ENTRY(nsICheckboxControlFrame)
 NS_QUERYFRAME_TAIL_INHERITING(nsFormControlFrame)

Can this be removed or simplified? I thought we had some shorthand for this

Comment 7

[Zack Weinberg (:zwol)]

Yanno, I think they can just be removed altogether.  It's NS_IMPL_FRAMEARENA_HELPERS that every single class has to have.  Will revise.

Comment 8

[Zack Weinberg (:zwol)]

Attachment: v3: remove unnecessary queryframe definitions

Like so.

Comment 9

[Zack Weinberg (:zwol)]

http://hg.mozilla.org/mozilla-central/rev/cc26a07db401