Last Comment Bug 315210 - Using munder:hover {display:-moz-box;} crashes [@ nsBox::SyncLayout] Mozilla
: Using munder:hover {display:-moz-box;} crashes [@ nsBox::SyncLayout] Mozilla
Status: VERIFIED FIXED
: crash, regression, testcase, verified1.8.0.1, verified1.8.0.4, verified1.8.1
Product: Core
Classification: Components
Component: MathML (show other bugs)
: Trunk
: x86 Windows XP
: -- critical (vote)
: ---
Assigned To: Bernd
: Hixie (not reading bugmail)
: Anthony Jones (:kentuckyfriedtakahe, :k17e)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-11-05 09:14 PST by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2006-08-15 12:44 PDT (History)
12 users (show)
dveditz: blocking1.8.1+
dveditz: blocking1.8.0.1+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (699 bytes, application/xhtml+xml)
2005-11-05 09:18 PST, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
patch (2.38 KB, patch)
2005-11-06 00:25 PST, Bernd
roc: review+
roc: superreview+
dveditz: approval1.8.0.1+
dveditz: approval1.8.1+
Details | Diff | Splinter Review
assert (1020 bytes, patch)
2005-11-07 11:21 PST, Bernd
roc: review+
roc: superreview+
dveditz: approval1.8.0.1+
dveditz: approval1.8.1+
Details | Diff | Splinter Review
backout (1020 bytes, patch)
2006-01-12 21:58 PST, Bernd
dveditz: approval1.8.0.1-
Details | Diff | Splinter Review
backout assert on the branch (483 bytes, patch)
2006-01-21 22:34 PST, Brian Ryner (not reading)
bernd_mozilla: review+
benjamin: approval‑branch‑1.8.1+
dveditz: approval1.8.0.2-
jaymoz: approval1.8.0.4+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2005-11-05 09:14:51 PST
See upcoming testcase, Mozilla crashes when hovering over the text.

Doesn't crash Mozilla1.7

Talkback ID: TB11483283Q

nsBox::SyncLayout  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBox.cpp, line 853]
nsFrame::DoLayout  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrame.cpp, line 5089]
nsIFrame::Layout  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBox.cpp, line 802]
nsBoxFrame::DoLayout  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1091]
nsContainerFrame::ReflowChild  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsContainerFrame.cpp, line 891]
nsMathMLContainerFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/mathml/base/src/nsMathMLContainerFrame.cpp, line 1086]
nsContainerFrame::ReflowChild  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsContainerFrame.cpp, line 891]
nsMathMLContainerFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/mathml/base/src/nsMathMLContainerFrame.cpp, line 1086]
nsLineLayout::ReflowFrame  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineLayout.cpp, line 996]
nsInlineFrame::ReflowInlineFrame  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsInlineFrame.cpp, line 681]
nsInlineFrame::ReflowFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsInlineFrame.cpp, line 511]
nsInlineFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsInlineFrame.cpp, line 425]
nsMathMLmathInlineFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/mathml/base/src/nsMathMLContainerFrame.h, line 445]
nsLineLayout::ReflowFrame  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineLayout.cpp, line 996]
nsBlockFrame::ReflowInlineFrame  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 4215]
nsBlockFrame::DoReflowInlineFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 3868]
nsBlockFrame::ReflowInlineFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 3741]
nsBlockFrame::ReflowLine  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 2736]
nsBlockFrame::ReflowDirtyLines  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 2270]
nsBlockFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 904]
nsBlockReflowContext::ReflowBlock  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockReflowContext.cpp, line 606]
nsBlockFrame::ReflowBlockFrame  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 3455]
nsBlockFrame::ReflowLine  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 2617]
nsBlockFrame::ReflowDirtyLines  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 2270]
nsBlockFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 904]
nsContainerFrame::ReflowChild  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsContainerFrame.cpp, line 891]
CanvasFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsHTMLFrame.cpp, line 525]
nsContainerFrame::ReflowChild  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsContainerFrame.cpp, line 891]
nsHTMLScrollFrame::ReflowScrolledFrame  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsGfxScrollFrame.cpp, line 521]
nsHTMLScrollFrame::ReflowContents  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsGfxScrollFrame.cpp, line 584]
nsHTMLScrollFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsGfxScrollFrame.cpp, line 781]
nsContainerFrame::ReflowChild  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsContainerFrame.cpp, line 891]
ViewportFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsViewportFrame.cpp, line 230]
IncrementalReflow::Dispatch  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 859]
PresShell::ProcessReflowCommands  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 6481]
ReflowEvent::HandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 6306]
PL_HandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/xpcom/threads/plevent.c, line 689]
SHELL32.dll + 0x520c24 (0x778b0c24)
Comment 1 Martijn Wargers [:mwargers] (not working for Mozilla) 2005-11-05 09:18:42 PST
Created attachment 201936 [details]
testcase
Comment 2 Bernd 2005-11-05 23:40:47 PST
This is combination of the code in MathML
http://lxr.mozilla.org/seamonkey/source/layout/mathml/base/src/nsMathMLTokenFrame.cpp#207

 FinishAndStoreOverflow(&aDesiredSize);
  // Act as if there is overflow no matter what. This is a 
  // safety measure to cater for math fonts with metrics that sometimes
  // cause glyphs in the text frames to protrude outside. Without this,
  // such glyphs may be clipped at the painting stage
  // This flag has already been set on the children as well in 
  // SetInitialChildList()
  mState |= NS_FRAME_OUTSIDE_CHILDREN; 

and after this we might end without a overflow property but a set flag

and
http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsBox.cpp#849

    if (GetStateBits() & NS_FRAME_OUTSIDE_CHILDREN) {
      nsRect* overflow = GetOverflowAreaProperty();
      NS_ASSERTION(overflow, "should have overflow area property");
      rect = *overflow;
    }

the only question is which place is more evil.
Comment 3 Bernd 2005-11-05 23:52:54 PST
I think the outliner code from bug 151375 is not robust enough.
Comment 4 Bernd 2005-11-06 00:25:31 PST
Created attachment 201982 [details] [diff] [review]
patch
Comment 5 rbs 2005-11-06 10:44:13 PST
The MathML's FinishAndStoreOverflow() was added in bug 151375 by aaronleventhal (attachment 153195 [details] [diff] [review]). I don't think it is doing anything useful there. It is way too early. We are still half-way into the reflow and the desired size isn't computed yet.

Anyway, the effect that MathML wants is described here:

http://lxr.mozilla.org/mozilla/source/layout/generic/nsContainerFrame.cpp#250

    nsRect damageArea;
    PRBool overlap;
    if (NS_FRAME_OUTSIDE_CHILDREN & aFrame->GetStateBits()) {
      // If the child frame has children that leak out of our box
      // then we don't constrain the damageArea to just the childs
      // bounding rect.
      damageArea = aDirtyRect;
      overlap = PR_TRUE;
    }


Constraining the damage area is the reason of bug 96041 and bug 79732 (see also dup bug 15116).

In the case of MathML (which we know for fact that it involves all sort of strange fonts prone to those issues), the outside bit is set to loosen the paint. It was very cheap and simple (and did not involved extra data). But now that an overflowArea property has been added in bug 151375, it will bloat MathML frames a lot if it is created on all MathML frames. So making the GetOverflow() null-safe as you doing in your patch is preferable on the long run. I still need to iterate on aaron's code, though.
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-11-06 12:26:21 PST
Setting NS_FRAME_OUTSIDE_CHILDREN without setting the overflow property is incorrect. The property has to be there. I don't mind adding the null-check in nsBox::SyncLayout but we should definitely still have the assertion and fix the code that triggers it.
Comment 7 rbs 2005-11-06 17:37:15 PST
>Setting NS_FRAME_OUTSIDE_CHILDREN without setting the overflow property is
>incorrect. The property has to be there.

Why do you think/want so? And what about the paint? Seems more robust/beneficial to me to relax the existence and null-safe in general, for case where the overflow can be computed on the fly, overloading |nsRect nsIFrame::GetOverflowRect()| -- which is not done yet, I might note. It will be a bloat if the property is created when we have the bounding metrics that already tell us the information needed at no extra cost.
Comment 8 Bernd 2005-11-07 11:21:15 PST
Created attachment 202125 [details] [diff] [review]
assert 

Robert, you are right we should catch the other "offenders" too.
Comment 9 Bernd 2005-11-07 11:37:30 PST
I added the assert, I thought it will fire up like hell but no, I could load all testcases at http://lxr.mozilla.org/seamonkey/source/layout/mathml/tests/ and it stayed silent (only some "minor" table paint asserts ...)

Given the code 
nsRect*
nsIFrame::GetOverflowAreaProperty(PRBool aCreateIfNecessary) 
{
  if (!((GetStateBits() & NS_FRAME_OUTSIDE_CHILDREN) || aCreateIfNecessary)) {
    return nsnull;
  }
i think the previous code 
 a) doubled the query
 b) differs from the code elsewhere
 c) did not deal with 0 robust enough.

I believe that Robert is right, what we see here is how fast (roughly 15 months) the consequences of such a hack surface. If a frame does not play by the rules then it should use its own paint routines and not use mechanisms that are used elsewhere.
Comment 10 rbs 2005-11-07 13:34:45 PST
Yep, roc is right. I wonder (not in this bug) if we shouldn't relax that and rely more on |nsRect nsIFrame::GetOverflowRect()|. That would be robust and work in general when the overflow is known without having to be stashed as a property. That will help the MathML situation (i.e., when it is made to work).
Comment 11 Worcester12345 2005-11-08 06:39:18 PST
Crashes branch:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051107 Firefox/1.5 ID:2005110708
Comment 12 Sébastien Auger 2005-11-08 10:22:32 PST
(In reply to comment #11)
> Crashes branch:
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051107
> Firefox/1.5 ID:2005110708
> 

we know but it will not make the 1.8 branch, only the trunk for now.
we can expect a patch to be taken for 1.8.1 maybe if it doesn't break anything.
Comment 13 Bernd 2005-11-10 11:23:41 PST
Robert, could you please explain, what I should change to get patch into the tree. 
Comment 14 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-11-10 17:54:19 PST
Sorry, I forgot to CC so I was not aware of the activity. Please check in both patches.

rbs,
> I wonder (not in this bug) if we shouldn't relax that and
> rely more on |nsRect nsIFrame::GetOverflowRect()|. That would be robust and
> work in general when the overflow is known without having to be stashed as a
> property. That will help the MathML situation (i.e., when it is made to work).

Yeah, we could make GetOverflowRect virtual and avoid stashing a property on MathML frames. But that's a tradeoff of course because it introduces a virtual call into the processing of non-MathML frames.

One question I have: how many MathML frames on average would actually have overflowing glyphs and need the overflow property? Not all, right? If the answer is that most MathML frames wouldn't need it, then I think the correct tradeoff is clearly to use the property and keep GetOverflowRect nonvirtual.

You probably already realize this but the existing hack can still fail in some situations. E.g. if a MathML frame is in a container frame of the same size, then the container will not mark itself as having outside children, and some paints won't reach the MathML frame.
Comment 15 Olli Pettay [:smaug] 2005-11-11 05:52:51 PST
Comment on attachment 201982 [details] [diff] [review]
patch


Why this?

>+        nsRect bounds = overflowArea ? *overflowArea + box->GetPosition() :
>+                                         bounds = box->GetRect();

Why not 
nsRect bounds = overflowArea ? *overflowArea + box->GetPosition() : box->GetRect();
Comment 16 rbs 2005-11-14 19:44:26 PST
Re: Comment #14:

>Yeah, we could make GetOverflowRect virtual and avoid stashing a property on
>MathML frames. But that's a tradeoff of course because it introduces a virtual
>call into the processing of non-MathML frames.

Seems a reasonnable trade-off. We already have tons of virtual functions... You can't be that picky on that one. [BTW, Knuth said in his TeXbook that the reason he used $...$ to delimit the math mode was because it turns out to be too expensive to typeset.]

>One question I have: how many MathML frames on average would actually have
>overflowing glyphs and need the overflow property? Not all, right? If the
>answer is that most MathML frames wouldn't need it, then I think the correct
>tradeoff is clearly to use the property and keep GetOverflowRect nonvirtual.

Italic glyphs (in the math identifier <mi>) are those that tend to protrude outside. Since all this is confined in <math>...</math>, which is often a little rect, it costs very little to paint the extra, if any.

>You probably already realize this but the existing hack can still fail in some
>situations. E.g. if a MathML frame is in a container frame of the same size,
>then the container will not mark itself as having outside children, and some
>paints won't reach the MathML frame.

That's possible indeed, but it won't be likely in the MathML case. There is a special thing that <math>...</math> always does. It pads itself with the italic correction (both left and right) so that it can stand out a bit from the surrounding text, without seeming too ostentatious at the same time (i.e., paints a bit more, if allowed). To see what I mean, try this test.xhtml to see, e.g., the difference between <i> and <mi>.


<?xml version="1.0"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1 plus MathML 2.0//EN" 
  "http://www.w3.org/TR/MathML2/dtd/xhtml-math11-f.dtd" [
  <!ENTITY mathml "http://www.w3.org/1998/Math/MathML">
]>
<html xmlns="http://www.w3.org/1999/xhtml">  
<head><title>Test</title></head>
<body>

|<i>j</i>|

|<math xmlns="&mathml;"><mi>j</mi></math>|

</body>
</html>
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-11-14 20:22:11 PST
(In reply to comment #16)
> Re: Comment #14:
> >Yeah, we could make GetOverflowRect virtual and avoid stashing a property on
> >MathML frames. But that's a tradeoff of course because it introduces a
> >virtualcall into the processing of non-MathML frames.
> 
> Seems a reasonnable trade-off. We already have tons of virtual functions...
> You can't be that picky on that one. [BTW, Knuth said in his TeXbook that the
> reason he used $...$ to delimit the math mode was because it turns out to be
> too expensive to typeset.]

I can so be picky! "We've always sucked" is not a good reason to suck a little bit more.

> >One question I have: how many MathML frames on average would actually have
> >overflowing glyphs and need the overflow property? Not all, right? If the
> >answer is that most MathML frames wouldn't need it, then I think the correct
> >tradeoff is clearly to use the property and keep GetOverflowRect nonvirtual.
> 
> Italic glyphs (in the math identifier <mi>) are those that tend to protrude
> outside. Since all this is confined in <math>...</math>, which is often a
> little rect, it costs very little to paint the extra, if any.

That doesn't really answer my question, though. I guess we'll have to instrument a build and find out.

> >You probably already realize this but the existing hack can still fail in
> >some situations. E.g. if a MathML frame is in a container frame of the same
> >size, then the container will not mark itself as having outside children, and 
> >some paints won't reach the MathML frame.
> 
> That's possible indeed, but it won't be likely in the MathML case. There is a
> special thing that <math>...</math> always does. It pads itself with the
> italic correction (both left and right) so that it can stand out a bit from
> the surrounding text, without seeming too ostentatious at the same time (i.e.,
> paints a bit more, if allowed).

So are you suggesting that all MathML frames set NS_FRAME_OUTSIDE_CHILDREN? I had thought it was just tokens.
Comment 18 rbs 2005-11-14 21:43:02 PST
> I had thought it was just tokens.

Yep, that's seem to be enough so far. I took the example of <math> because it is the one likely to be inside a foreign fame beyond control.

But there are other oddities like bug 161155, making me wonder what is going on.
Comment 19 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-11-14 22:13:08 PST
It's probably the same thing. If you have an overflowing glyph in a token frame (or any other frame for that matter) with NS_FRAME_OUTSIDE_CHILDREN, but no overflow rect, inside a MathML container frame (not <math>), then the container frame will not get NS_FRAME_OUTSIDE_CHILDREN even though the glyph may spill out of it. Right?
Comment 20 rbs 2005-11-14 22:52:16 PST
That's right, only token frames are setting the bit now. But it still quite suprising that it clips this bad (on that bug)...
Comment 21 Bernd 2005-12-03 10:23:51 PST
both patches got checked in 2005-11-10 21:30
Comment 22 Stephen Donner [:stephend] 2005-12-04 15:35:51 PST
Verified FIXED using trunk build 2005-12-04-10 of SeaMonkey on Windows XP with the testcase at: https://bugzilla.mozilla.org/attachment.cgi?id=201936.  No crash.
Comment 23 Bernd 2005-12-23 01:15:16 PST
Comment on attachment 201982 [details] [diff] [review]
patch

This could go on branch, its pretty secure
Comment 24 Daniel Veditz [:dveditz] 2006-01-05 11:32:45 PST
Comment on attachment 202125 [details] [diff] [review]
assert 

a=dveditz for 1.8/1.8.0.1 branches. Please add the fixed1.8.1 and fixed1.8.0.1 keywords when checked in.
Comment 25 Daniel Veditz [:dveditz] 2006-01-05 11:33:12 PST
Comment on attachment 201982 [details] [diff] [review]
patch

a=dveditz for 1.8/1.8.0.1 branches. Please add the fixed1.8.1 and fixed1.8.0.1 keywords when checked in.
Comment 26 Bernd 2006-01-07 13:10:18 PST
fixed on branches
Comment 27 Scott MacGregor 2006-01-09 15:28:37 PST
bernd, I'm unable to run thunderbird on the branch after this change. 
I get (what seems like but propably isn't) a very large number of assertions hitting this code.

Frame abuses NS_FRAME_OUTSIDE_CHILDREN flag.

It's possible we're trying to bring up the extension compatibility checker dialog instead of the thunderbird mail chrome. I haven't figured out which one yet.

I don't see this on the trunk oddly enough.

Comment 28 Scott MacGregor 2006-01-09 15:30:51 PST
I am able to eventually get passed all the assertions here. I see it with any window I try bring up: extensions compatibilty, 3-pane, mail compose, extension manager, theme manager etc.

This seems like a pretty bad regression from this bug, it makes debug builds extremely painful to use on the branch. 
Comment 29 Bernd 2006-01-09 21:24:29 PST
I am not religous about the assert, it would however be cool to post a stack trace and then to back the patch out, as long as the ill-doing frame is still on branch
So post a stack and back the thing out if it hurts your work.
Comment 30 Scott MacGregor 2006-01-10 13:51:45 PST
Well before we back out the assertion. Does the assertion really mean something unexpected or bad has happened? I'm assuming it was added there for a reason. Yet most of our chrome windows (not just Thunderbird's) set this assertion off. Does that mean someone is making wrong assumptions about something?
Comment 31 Jay Patel [:jay] 2006-01-10 15:31:44 PST
v.fixed on 1.8.0.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060109 Firefox/1.5.0.1, no crash using testcase.

I'll leave it up to you guys to deal with the assertions.  Perhaps we need a new bug for that regression?
Comment 32 Bernd 2006-01-10 21:30:01 PST
The assert means, a frame we don't know yet which, but we should know the fellow, sets the NS_FRAME_OUTSIDE_CHILDREN flag without setting the overflow area. 
This is bad because the frame tries to get painted beyond its borders because there are some flaws in its size computation that lead to clipped painting otherwise. This band aid hack is usually nixed if this frame is nested into another frame because it will see no overflow from the bad fellow and don't set the NS_FRAME_OUTSIDE_CHILDREN for itself. So the painting will be clipped at the parent level.

As Scott indicates this is allready fixed on trunk so I really think, one should  indicate which frame is it and back it out.
Comment 33 Bob Clary [:bc:] 2006-01-12 14:06:05 PST
fwiw I see Frame abuses NS_FRAME_OUTSIDE_CHILDREN assert 12 times justing starting Firefox 1.5.0
Comment 34 Scott MacGregor 2006-01-12 14:11:20 PST
I'm glad I'm not the only one getting bogged down by the assertion!

Bernd since you added this assertion, can you drive it to resolution one way or the other? Just start up firefox or thunderbird and you should see it enough times to get a good stack trace. 
Comment 35 Bernd 2006-01-12 21:58:10 PST
Created attachment 208353 [details] [diff] [review]
backout

Please let me backout this thing. This are things that need to get the trunk in shape not a branch punishment.  We are sorry for the inconvenience...
Comment 36 Daniel Veditz [:dveditz] 2006-01-17 11:06:23 PST
Comment on attachment 208353 [details] [diff] [review]
backout

Not for 1.8.0.1, probably 1.8.0.2

Is this patch reversed? This shows you adding yet another assertion.
Comment 37 Brian Ryner (not reading) 2006-01-21 22:34:37 PST
Created attachment 209257 [details] [diff] [review]
backout assert on the branch

Bernd, can we just remove the assertion on the branch for now?  This is pretty much making it impossible for me to debug anything on the branch.
Comment 38 Bernd 2006-01-21 23:40:15 PST
Comment on attachment 209257 [details] [diff] [review]
backout assert on the branch

Sure, back this out, I would have done it for a long timne, but I did not get the permission to do so.
Comment 39 Brian Ryner (not reading) 2006-01-23 14:32:50 PST
assertion backed out on MOZILLA_1_8_BRANCH
Comment 40 Ben Turner (not reading bugmail, use the needinfo flag!) 2006-02-22 12:41:22 PST
Comment on attachment 209257 [details] [diff] [review]
backout assert on the branch

It would be great to get this out of the 1.8.0 branch also.
Comment 41 Daniel Veditz [:dveditz] 2006-02-23 11:51:45 PST
Comment on attachment 209257 [details] [diff] [review]
backout assert on the branch

wait for 1.8.0.3 please
Comment 42 Jay Patel [:jay] 2006-04-05 11:55:36 PDT
Comment on attachment 209257 [details] [diff] [review]
backout assert on the branch

Please check in promptly on the 1.8.0 branch.  Thanks!
Comment 43 Bernd 2006-04-06 12:57:22 PDT
fixed on the 1.8.0 branch

Note You need to log in before you can comment on or make changes to this bug.