When display:none is set on a toolbar (by clicking on the grippy and setting the attribute via CSS), the reflow is triggered but the frames in question never get destroyed.

On debug builds, this is easy to test. Click on the grippy in a toolbar, and you will see the following message if this still happens: BUG 3505:: Found a collapsed toolbar (display:none) but the frame still exists!!!! I'm testing in the reflow code if the attribute has been set on the content node. The printf occurs if there is a frame that corresponds to the content node and the attribute is set. It shouldn't trigger the printf because there shouldn't be a frame for that content node, but it does, ergo the bug.

Setting m4 milestone and marking it as a blocker

adding trudelle to cc list

added beppe to cc list

*** Bug 3538 has been marked as a duplicate of this bug. ***

Accepting bug. I will look into this today.

Yeah I'm seeing this on Linux now.

I'm seeing this on Linux, too, and don't necessarily have to click on a grippy to get it (pnunn was playing around with loading images in apprunner and we saw a bunch of these messages).

Any progress on this bug Nisheeth? It is still blocking Pink from work we need on the toolbar.

OK, I am on this as I write this. Last week I got diverted into M3 bugs. Sorry about that, pink. I'll update this bug as I know more...

nsToolboxFrame::CollapseToolbar() handles the click event on the grippy on the toolbar. It calls RDFElementImpl::SetAttribute() to set the "collapsed" attribute on the toolbar to true. RDFElementImpl::SetAttribute() sends an attribute change notification that ends up at nsCSSFrameConstructor::AttributeChanged() which calls RDFElementImpl::GetStyleHintForAttributeChange() to get a style hint from the RDF content object. In order for collapsing toolbars to work the following needs to happen: 1) RDFElementImpl::GetStyleHintForAttributeChange() should return a style hint of NS_STYLE_HINT_FRAMECHANGE if the "collapsed" attribute is changing. I think its going to be bad to centralize the style hint decision inside the RDF element. Maybe it should get a style hint from the XUL element contained within it. That way each XUL element gets to decide the style hint for attribute changes on itself. I'm re-assigning the bug to Chris Waterson, owner of RDFElementImpl and putting myself on the cc list.

The node shouldn't have to have any special knowledge about this to trigger a reframe. I can make up any attribute i want and have style attribute selectors that reframe, or reflow, or make it dance like a chicken if the style I write says so. Or am i totaly misunderstanding the general nature of attributes and css?

Accepted bug to get of terry's spambot.

Eli, I'm re-assigning you these Mac bugs to take ownership. These are Greg's old bugs...

Any status on this? I'm blocked on this and its target milestone is M4.

Added hyatt to cc list. He understands all this layout stuff. I don't.

Nisheeth, you should talk with Peter Linss about how the attribute system works. FOR ANY ATTRIBUTE CHANGES THAT CAUSE ATTRIBUTE SELECTOR-BASED CSS RULES TO BE MATCHED/UNMATCHED, THE ATTRIBUTE HINT SYSTEM IS IRRELEVANT. Since Pink's toolbars rely only on a CSS attribute-selector rule that happens to specify a reframe (it could just as easily have only needed a reflow), it is up to the style system code that fires on an attribute changed event to determine the impact of the change caused by the newly matched/unmatched rules and to do the right thing. We should NOT be returning the hint that you're saying we should be returning. That's wrong.

This sounds like it's Peter Linss's bug.

Yes, I agree with Pink and Hyatt's analysis. I was confused.

Slipping to M5.

I believe this is fixed with my checkin for bug 4622. Anyone care to check?

Re-assigning this to Eric Vaughan, as per discussion between Vidur, him, and me. nsBoxFrame needs to over-ride the InsertFrame() and RemoveFrame() methods of nsIFrame.

Ok I looked at this. nsBoxFrame does not respond when frames are removed or added dynamically because its base class nsContainerFrame does not provide any default behavior for RemoveFrames, AppendFrames, InsertFrames. This class should provide basic implementation so subclasses can inherit them. If I just put this code in my nsBoxFrame I would have to duplicate the code over and over for every frame I create that is a container. Only frames that need to do something special like BlockFrames should be redefining these methods. Kipp can you take a look at this and maybe to get to the right layout person? Thanks

After a bit of debate, we ended up deciding that because the desired behavior isn't (yet) generic, eric would put the necessary code in his container class(s).

Implemented InsertFrames, AppendFrames, and DeleteFrame. All fixed.

Using the current Win32 and Linux M4 builds (4.13.99), I can't reproduce this problem. However, I'd prefer that Mike put in his two cents prior to verifying it, since I'm not familiar with the functional area, and am relying on the StdOut warning text.

It works with the top toolbox, but not the blue toolbar at the bottom. I think that there is still something wrong with the css reflow analysis. I'll put in a breakpoint and see if DeleteFrame is being called on the box. If not, it's a CSS bug.

Ok, today it doesn't work at all. RemoveFrame isn't being called on any toolbox. reopening bug.

clearing the resolution.

This worked for a while. But now in the current build removeFrame doesn't ever called anymore.

Don't know why this isn't working, but layout ain't my bag, baby!

changing platform to all Ok, we're back to the state we were in a week or two ago. Collapsing in the top toolbox in apprunner works (as much as I've coded it, the toolbars disappear, which is the key). However, in the toolbox at the bottom of the window (the blue one, representing a task bar) the reframe still does not happen. The nsBoxFrame::RemoveFrames() method is not getting called where is most certainly is getting called in the top toolbox. As far as the bottom toolbox knows, it is setting the attribute which triggers the style change to display:none. In debug builds it continues to print out diagnostic error messages that the frame is still there when the attribute is set to be collapsed. Why this works in one toolbox and not the other is the new bug. Let's keep this one open because the refame-determination code isn't perfect.

It turned out to be css cascading priority bug with xul.css and navigator.css...fixed!

verifying.

Thanks, Mike!