Remove some unused box object related interfaces

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

12 years ago
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.
(Assignee)

Comment 1

12 years ago
Created attachment 225273 [details] [diff] [review]
something like this

Perhaps Neil has some comments to this :)
Assignee: nobody → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #225273 - Flags: review?(neil)
(Assignee)

Comment 2

12 years ago
Created attachment 225276 [details] [diff] [review]
part 2 of the patch

This was missing from the patch

Comment 3

12 years ago
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.
(Assignee)

Comment 5

12 years ago
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.  :(
(Assignee)

Comment 7

12 years ago
Comment on attachment 225273 [details] [diff] [review]
something like this

Ok, I'll try someone else...
Attachment #225273 - Flags: review?(bzbarsky)
(Assignee)

Updated

12 years ago
Attachment #225273 - Flags: review?(roc)
(Assignee)

Comment 8

12 years ago
Created attachment 225416 [details] [diff] [review]
v2

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?
(Assignee)

Comment 10

12 years ago
Created attachment 228998 [details] [diff] [review]
Keeping interfaces for browser, iframe and editor
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+
(Assignee)

Updated

12 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Blocks: 340510

Updated

10 years ago
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.