545156 - Expose nsPopupSetFrame's ::popupList

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Mats Palmgren (inactive)]

Spawned off from bug 544251.

We should expose nsPopupSetFrame's ::popupList by implementing
GetChildList(), GetAdditionalChildListName() for nsPopupSetFrame.

(and uncomment the NS_ASSERTION in SetInitialChildList())

Comment 1

[Neil Deakin]

Attachment: Patch

This implements GetChildList and GetChildLists. Is there more to implement here?

Comment 2

[Mats Palmgren (inactive)]

Comment on attachment 812920 [details] [diff] [review]
Patch

> Is there more to implement here?

Not that I'm aware of...

We do handle kPopupList in few places already though, for example:
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#1718
I suspect those places are tailored for nsMenuFrame which also
use kPopupList, can you check that they seem reasonable also
for nsPopupSetFrame?

Comment 3

[Neil Deakin]

Attachment: Patch version 2, as suggested

This makes some additional changes as suggested.

The only case I wasn't sure about was the use of kPopupList in RestyleManager.cpp::SyncViewsAndInvalidateDescendants

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 815864 [details] [diff] [review]
Patch version 2, as suggested

I think we can also remove nsPopupSetFrame::List since the inherited
List method (nsContainerFrame) should now print this list.

> nsLayoutUtils.cpp
> +      if (parent) {

MOZ_ASSERT(parent, "nsMenuPopupFrame can't be the root frame");


> +        }
> +        else {

} else {


> } else if (nsGkAtoms::tableCaptionFrame == aChildFrame->GetType()) {

s/aChildFrame->GetType()/childType/  while we're here...


r=mats

Comment 5

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/214adf185651

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/214adf185651