Closed
Bug 322436
Opened 17 years ago
Closed 15 years ago
WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with float cache
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: roc)
References
Details
(4 keywords, Whiteboard: required by 366128)
Attachments
(4 files, 1 obsolete file)
341 bytes,
text/html
|
Details | |
201 bytes,
text/html
|
Details | |
1.29 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
2.58 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
Steps to reproduce: 1. Load the testcase in a trunk debug build. Result: ###!!! ASSERTION: not in child list: 'nsFrameList(aChildFrame->GetParent()->GetFirstChild(listName)) .ContainsFrame(aChildFrame)', file /Users/admin/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 1888
Reporter | ||
Comment 1•17 years ago
|
||
Reporter | ||
Comment 2•17 years ago
|
||
bz, here's one of the testcases you asked for in bug 319280 comment 13.
![]() |
||
Comment 3•17 years ago
|
||
This testcase shows what I think is the underlying problem -- the float being dropped from the frame tree altogether. I'll bet money the reason is that BuildFloatList doesn't walk into boxes and that the answer is to nuke BuildFloatList.
Reporter | ||
Comment 4•16 years ago
|
||
I don't see the assertion any more (after BuildFloatList removal), but I do see WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with float cache: file /Users/admin/trunk/mozilla/layout/generic/nsBlockFrame.cpp, line 7068 and the second testcase's "This text should show up" text still doesn't show up.
Assignee | ||
Comment 5•16 years ago
|
||
Yeah, we're still not reflowing the float. However, I believe we have changed the error from something potentially catastrophic to something merely incorrect.
Reporter | ||
Comment 6•16 years ago
|
||
That's good to know. Each warning is accompanied by a screenful of frame dumps (?), so if I see the warning often while fuzzing, I'll ask you to make at least the frame dump be DEBUG_roc or something.
Comment 7•16 years ago
|
||
(In reply to comment #6) > I'll ask you to make at least the frame dump be DEBUG_roc or something. I'll second that request.
Attachment #224570 -
Flags: superreview?(roc)
Attachment #224570 -
Flags: review?(roc)
Assignee | ||
Updated•16 years ago
|
Attachment #224570 -
Flags: superreview?(roc)
Attachment #224570 -
Flags: superreview+
Attachment #224570 -
Flags: review?(roc)
Attachment #224570 -
Flags: review+
Comment 8•16 years ago
|
||
Comment on attachment 224570 [details] [diff] [review] Patch to mute the frame dumps Landed on trunk.
Comment 9•16 years ago
|
||
A float inside a Box where the float containing block is an ancestor of the Box is going to trigger this warning. So, what is the real fix to this bug? do we want to add the float of the nearest block reflow state? or wrap it in a anonymous block during frame construction? or something else? In the meantime, we could make CheckFloats() be silent for this particular case, since it's a known problem (allowing us to see any remaining issues with the float cache).
OS: Mac OS X 10.2 → All
Hardware: Macintosh → All
Summary: ASSERTION: not in child list: 'nsFrameList(aChildFrame->GetParent()->GetFirstChild(listName)) .ContainsFrame(aChildFrame)' → WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with float cache
Assignee | ||
Comment 10•16 years ago
|
||
You mean a XUL box?
Comment 11•16 years ago
|
||
Yes - anything that uses nsFrame::BoxReflow() http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsFrame.cpp&rev=3.667&root=/cvsroot&mark=5534#5532 It doesn't seem to add the float to the block reflow state (which eventually adds it to the float cache for the line), like nsLineLayout::ReflowFrame does: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineLayout.cpp&rev=3.238&root=/cvsroot&mark=1013-1018#992 There is also this block: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsFrame.cpp&rev=3.667&root=/cvsroot&mark=5657-5659#5648 which made me think it could be intentional.
Assignee | ||
Comment 12•16 years ago
|
||
I don't know how that should be handled. Perhaps we should just ignore the float property on elements that aren't in a block formatting context?
Assignee | ||
Comment 13•16 years ago
|
||
A fix for this could be backported to the 1.0.x branch, which might be useful. I'll try to work on it.
Reporter | ||
Comment 14•16 years ago
|
||
Nominating because this blocks an [sg:critical] crash bug.
Flags: blocking1.9?
Assignee | ||
Updated•15 years ago
|
Flags: blocking1.9? → blocking1.9+
Comment 15•15 years ago
|
||
Is this ready for check-in on the trunk? the problem blocks some of jesse's testing. also do we know if it really does fix bug 366128, or will more work be needed for that one.
Reporter | ||
Comment 16•15 years ago
|
||
The patch in this bug was just to reduce the amount of console output when this bug occurs. There isn't a patch for this bug yet afaik.
Assignee | ||
Comment 17•15 years ago
|
||
This is basically the same as bug 366128. We should fix it by pushing a null float containing block for XUL box frames, as Boris suggests: https://bugzilla.mozilla.org/show_bug.cgi?id=366128#c10
Assignee | ||
Comment 18•15 years ago
|
||
It should be this simple.
Attachment #266862 -
Flags: superreview?(bzbarsky)
Attachment #266862 -
Flags: review?(bzbarsky)
![]() |
||
Comment 19•15 years ago
|
||
Comment on attachment 266862 [details] [diff] [review] fix Would that it were so. GetFloatContainingBlock also needs to be changed.
Attachment #266862 -
Flags: superreview?(bzbarsky)
Attachment #266862 -
Flags: superreview-
Attachment #266862 -
Flags: review?(bzbarsky)
Attachment #266862 -
Flags: review-
Assignee | ||
Comment 20•15 years ago
|
||
Indeed.
Attachment #266862 -
Attachment is obsolete: true
Attachment #266892 -
Flags: superreview?(bzbarsky)
Attachment #266892 -
Flags: review?(bzbarsky)
![]() |
||
Comment 21•15 years ago
|
||
Comment on attachment 266892 [details] [diff] [review] fix v2 I assume you checked that IsBoxFrame() is true if and only if the frame is created by ConstructXULFrame, right? If so, r+sr=bzbarsky.
Attachment #266892 -
Flags: superreview?(bzbarsky)
Attachment #266892 -
Flags: superreview+
Attachment #266892 -
Flags: review?(bzbarsky)
Attachment #266892 -
Flags: review+
Reporter | ||
Updated•15 years ago
|
Assignee: nobody → roc
Assignee | ||
Comment 22•15 years ago
|
||
There was one issue --- ConstructXULFrame creates non-box blocks for <label>, <text> and <description>. So I added an IsBoxFrame check here: + nsFrameConstructorSaveState floatSaveState; + if (newFrame->IsBoxFrame()) { + aState.PushFloatContainingBlock(nsnull, floatSaveState, PR_FALSE, + PR_FALSE); + } and checked it in.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Flags: blocking1.8.1.6?
Whiteboard: required by 366128
Comment 23•15 years ago
|
||
Mats incorporated this into a branch fix for bug 384344
Keywords: fixed1.8.0.13,
fixed1.8.1.5
Updated•15 years ago
|
Flags: blocking1.8.1.6?
Reporter | ||
Comment 24•15 years ago
|
||
I committed my testcase as a crashtest. I also committed bz's testcase as a reftest. I assumed the current rendering is correct.
Flags: in-testsuite+
Updated•4 years ago
|
Product: Core → Core Graveyard
Updated•4 years ago
|
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in
before you can comment on or make changes to this bug.
Description
•