Delete vestigial nsICheckboxControlFrame and nsIRadioControlFrame interface classes

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
13 years ago
10 years ago

People

(Reporter: jlurz24, Assigned: zwol)

Tracking

(Blocks 1 bug)

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Reporter

Description

13 years ago
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
Reporter

Comment 1

13 years ago
Posted patch patch_v1 (obsolete) — Splinter Review
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.
Attachment #230388 - Flags: review?(roc)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter

Comment 2

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

Updated

10 years ago
Blocks: deCOM
Assignee

Comment 3

10 years ago
Hm, we have a reviewed patch that never landed, here.  I might try to dust it off; reassigning to myself.
Assignee: nobody → zweinberg
Status: NEW → ASSIGNED
Assignee

Comment 4

10 years ago
Posted patch revised patch (obsolete) — Splinter Review
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.
Attachment #230388 - Attachment is obsolete: true
Attachment #425935 - Flags: review?(roc)
Assignee

Comment 5

10 years ago
Posted patch actually revised patch (obsolete) — Splinter Review
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.
Attachment #425935 - Attachment is obsolete: true
Attachment #426001 - Flags: review?(roc)
 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
Assignee

Comment 7

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

Comment 8

10 years ago
Like so.
Attachment #426001 - Attachment is obsolete: true
Attachment #426065 - Flags: review?(roc)
Attachment #426001 - Flags: review?(roc)
Assignee

Comment 9

10 years ago
http://hg.mozilla.org/mozilla-central/rev/cc26a07db401
Summary: DeCOMtaminate nsICheckboxControlFrame → Delete vestigial nsICheckboxControlFrame and nsIRadioControlFrame interface classes
Assignee

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.