Closed
Bug 345681
Opened 19 years ago
Closed 15 years ago
Delete vestigial nsICheckboxControlFrame and nsIRadioControlFrame interface classes
Categories
(Core :: Layout: Form Controls, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jlurz24, Assigned: zwol)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 3 obsolete files)
15.45 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
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
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)
Updated•19 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #230388 -
Flags: superreview+
Attachment #230388 -
Flags: review?(roc)
Attachment #230388 -
Flags: review+
Thanks Roc! I did notice that bz was making changes to this file on the reflow branch, so we should coordinate with him.
Assignee | ||
Comment 3•15 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•15 years ago
|
||
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)
Attachment #425935 -
Flags: review?(roc) → review+
Assignee | ||
Comment 5•15 years ago
|
||
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•15 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•15 years ago
|
||
Like so.
Attachment #426001 -
Attachment is obsolete: true
Attachment #426065 -
Flags: review?(roc)
Attachment #426001 -
Flags: review?(roc)
Attachment #426065 -
Flags: review?(roc) → review+
Assignee | ||
Comment 9•15 years ago
|
||
Summary: DeCOMtaminate nsICheckboxControlFrame → Delete vestigial nsICheckboxControlFrame and nsIRadioControlFrame interface classes
Assignee | ||
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•