Reflow branch XUL submenu issues

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Reflow branch XUL submenu issues

Boris Zbarsky
So I was poking around at why submenus of XUL context menus don't appear on
reflow branch (see attached testcase).

It looks like when we call PresShell::FrameNeedsReflow() on the menupopup, we
bail out early because the nsMenuFrame has NS_FRAME_HAS_DIRTY_CHILDREN set.  But
mDirtyRoots is empty, so the following flush doesn't have any effect.  The code
in nsMenuFrame where this is happening is:

       if (!wasOpen)
       {
          menuPopup->AddStateBits(NS_FRAME_IS_DIRTY);
          presContext->PresShell()->
            FrameNeedsReflow(menuPopup, nsIPresShell::eStyleChange);
          presContext->PresShell()->FlushPendingNotifications(Flush_OnlyReflow);
       }

in nsMenuFrame::OpenMenuInternal, and we have NS_FRAME_HAS_DIRTY_CHILDREN set
when we enter that function....

It seems odd to me that we can have a frame with NS_FRAME_HAS_DIRTY_CHILDREN set
while mDirtyRoots is empty and we're not in the middle of reflow.  Or can this
happen in some sane way?

-Boris

_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: Reflow branch XUL submenu issues

Boris Zbarsky
Boris Zbarsky wrote:
> It seems odd to me that we can have a frame with
> NS_FRAME_HAS_DIRTY_CHILDREN set while mDirtyRoots is empty and we're not
> in the middle of reflow.  Or can this happen in some sane way?

OK, this is in fact happening in an insane way.  We're messing with a kid of the
popup, which marks things dirty up to the reflow root or frame tree root.  The
parent of the popup is the same as the parent of its placeholder, so we just
mark things dirty up to the viewport.  But reflowing the viewport does NOT
reflow the popup.  The only way to get the popup to reflow is to either reflow
it directly (as a reflow root), or to reflow the popup set.  This used to be
handled by nsMenuPopupFrame::RelayoutDirtyChild calling up to the popupset, but
that code is now gone.

Setting the popup as a reflow root causes asserts in nsHTMLReflowState, since
it's an unknown type of out-of-flow.  So I think the right thing to do is to
either set the popupset as the GetParent() of the menupopup and having the
popupset be a reflow root or to make the menupopup a reflow root and fix the
reflow state to deal (or both!).

Anyone see any obvious issues with the GetParent() change?  It should be less
invasive, since it keeps popups reflowing as XUL boxes...

-Boris
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: Reflow branch XUL submenu issues

L. David Baron
On Wednesday 2006-08-02 21:50 -0500, Boris Zbarsky wrote:
> Setting the popup as a reflow root causes asserts in nsHTMLReflowState,
> since it's an unknown type of out-of-flow.  So I think the right thing to

Is there any good reason popups shouldn't be reflow roots?  If not, then
they should be.

-David

--
L. David Baron                                <URL: http://dbaron.org/ >
           Technical Lead, Layout & CSS, Mozilla Corporation

_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Reflow branch XUL submenu issues

Boris Zbarsky
In reply to this post by Boris Zbarsky
L. David Baron wrote:
> On Wednesday 2006-08-02 21:50 -0500, Boris Zbarsky wrote:
>> Setting the popup as a reflow root causes asserts in nsHTMLReflowState,
>> since it's an unknown type of out-of-flow.  So I think the right thing to
>
> Is there any good reason popups shouldn't be reflow roots?  If not, then
> they should be.

Conceptually, that would make a lot of sense to me.  Is reflowing an nsBoxFrame
equivalent to calling Layout() on the corresponding box?  In any case, I'll try
doing this.

Speaking of popups, it looks like the #if 0 in
nsMenuFrame::GetAdditionalChildListName got removed on branch.  This causes
asserts about things still being in the primary frame map when they're being
destroyed...  The real issue seems to be that nsMenuFrame::Destroy changes
attributes on the menupopup, which resets the primary frame for the menupopup,
after which we call DestroyPopupFrames() and destroy the frame we just readded
to the frame map.  Not sure why the GetAdditionalChildListName code is really
relevant here, but skipping over nsLayoutAtoms::popupList in
DoDeletingFrameSubtree seems to help.  Not sure why it does.  Mats, you probably
know this code best; any ideas on it?

-Boris
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: Reflow branch XUL submenu issues

Boris Zbarsky
Boris Zbarsky wrote:
>> Is there any good reason popups shouldn't be reflow roots?  If not, then
>> they should be.
>
> Conceptually, that would make a lot of sense to me.  Is reflowing an
> nsBoxFrame equivalent to calling Layout() on the corresponding box?  In
> any case, I'll try doing this.

This does seem to work; I'll land it on the branch.  But should we still set the
parent of the popups to the popupset?  Or leave things as they are now?

-Boris
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout