Closed Bug 341245 Opened 16 years ago Closed 16 years ago

Remove some unused box object related interfaces

Categories

(Core :: XUL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: smaug, Assigned: smaug)

References

Details

Attachments

(1 file, 3 obsolete files)

There are some interfaces in layout/xul/base/public which aren't used at all.
for example nsIBoxLayoutManager and nsIBoxPaintManager.
I think those can be removed.

nsIBrowserBoxObject, nsIIFrameBoxObject and nsIEditorBoxObject have all just
one attribute, |readonly attribute nsIDocShell docShell|
IMO, those could be merged to nsIDocShellContainer or whatever would be a good
name.
Attached patch something like this (obsolete) — Splinter Review
Perhaps Neil has some comments to this :)
Assignee: nobody → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #225273 - Flags: review?(neil)
Attached patch part 2 of the patch (obsolete) — Splinter Review
This was missing from the patch
Comment on attachment 225273 [details] [diff] [review]
something like this

After the mess of bug 340084 I don't feel qualified to review this. However, I'd name the class nsContainerBoxObject etc. I'd also use FORWARD_NSICONTAINERBOXOBJECT(nsBoxObject::)
Attachment #225273 - Flags: review?(neil)
> one attribute, |readonly attribute nsIDocShell docShell|

Which I'd like to go away in favor of an nsIWebNavigation, frankly....  any time our chrome uses nsIDocShell, we have a bug, imo.
Comment on attachment 225273 [details] [diff] [review]
something like this

Boris, do you want to say something else about this.
I agree with Neil that nsIContainerBoxObject would be a better name.

Changing the attribute to nsIWebNavigation is a different bug. (But I'll investigate that too :) )
Attachment #225273 - Flags: review?(bzbarsky)
smaug, I'm pretty totally swamped right now; I won't be able to do a detailed review of something like this probably until July.  :(
Comment on attachment 225273 [details] [diff] [review]
something like this

Ok, I'll try someone else...
Attachment #225273 - Flags: review?(bzbarsky)
Attachment #225273 - Flags: review?(roc)
Attached patch v2 (obsolete) — Splinter Review
This contains both patches, uses name ContainerBoxObject and ::GetDocShell
moved to ContainerBoxObject since it was not needed in nsBoxObject.
The previous patch was also missing some deleted files; this one has quite good
+/- ratio :) +261/-837 lines
Attachment #225273 - Attachment is obsolete: true
Attachment #225276 - Attachment is obsolete: true
Attachment #225416 - Flags: review?(roc)
Attachment #225273 - Flags: review?(roc)
This is actually a XUL API change. People who were using the old nsIEditorBoxObject etc interfaces will be broken. I suspect that's a bad idea.

How about we define nsIEditorBoxObject, nsIBrowserBoxObject, and nsIIFrameBoxObject, make them all just inherit from nsIContainerBoxObject adding no new methods, and then implement them as tearoffs on nsContainerBoxObject?
Attachment #225416 - Attachment is obsolete: true
Attachment #228998 - Flags: review?(roc)
Attachment #225416 - Flags: review?(roc)
Comment on attachment 228998 [details] [diff] [review]
Keeping interfaces for browser, iframe and editor

OK.

Removing the layout and paint managers is technically an interface change, but since no objects implementing those interfaces ever existed, I think we'll be OK.
Attachment #228998 - Flags: superreview+
Attachment #228998 - Flags: review?(roc)
Attachment #228998 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Blocks: 340510
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.