nsPopupSetFrame uses a custom frame list class, nsPopupFrameList, for its ::popupList children. It looks like the features it provides isn't used anymore and can be replaced by nsFrameList. The features are: * the child frames in a nsPopupFrameList has its prev/next-sibling pointers set to nsnull * it seems to support having nsIContent pointers without a corresponding frame, and "filling in" the frame at a later point None of the above seems to be required/used anymore. Another thing that is special about ::popupList is that calling nsPopupSetFrame::GetChildList(nsGkAtoms::popupList) always returns an empty list. This causes nsFrameConstructorState::ProcessFrameInsertions to call SetInitialChildList() when there are existing child frames when it should have called Append/InsertFrames depending on tree position for the child content. Currently, calling nsPopupSetFrame::SetInitialChildList() *prepends* the frames to the list, so calling it multiple times is sort of working. BTW, both AppendFrames and InsertFrames does the same. All three just calls AddPopupFrameList(): http://mxr.mozilla.org/mozilla-central/source/layout/xul/base/src/nsPopupSetFrame.cpp#272

BTW, is there any reason we didn't expose ::popupList by implementing GetChildList(), GetAdditionalChildListName() ?

Comment on attachment 425204 [details] [diff] [review] Patch rev. 1 I'd really like Neil and roc to look this over. I seem to recall that exposing is not good in terms of hit-testing behavior and the like (as in, we have consumers who absolutely depend on it not being exposed). I'm not sure why we needed the frames not linked and such, so can't judge whether it's ok to relink them.

Hit-testing won't be a problem anymore (if it ever was), because for a frame to show up in hit-testing you have to traverse it during BuildDisplayList. Just adding it to GetAdditionalChildListName won't do anything. Off the top of my head I don't know of any reason to not expose it.

If Neil doesn't know of a reason, then I think we should go ahead and expose it.

I'd be fine with that if we give the GetAdditionalChildListName consumers a brief look in the process.

Comment on attachment 425204 [details] [diff] [review] Patch rev. 1 I think this patch is fine though

Filed bug 545156 about exposing ::popupList http://hg.mozilla.org/mozilla-central/rev/e6d3799ef997