Closed Bug 920158 Opened 6 years ago Closed 6 years ago
CSSFrame Constructor::Construct Frame From Item Internal handle frames that should suppress floating of descendants .
No description provided.
FWIW this assert was added due to a request from bz here: https://bugzilla.mozilla.org/show_bug.cgi?id=666041#c45
Maybe we should really move this assert *inside* of PushFloatContainingBlock()? That would be extending its effect, but in a useful way.
Ah. Now I recall what the point of this assert is. Thank you for the link in comment 2. If ShouldSuppressFloatingOfDescendants() then pushing nothing or pushing the frame is wrong. The right thing to do is to push null. So if we can have such frames coming through here now we should remove the assert and do something more like what nsCSSFrameConstructor::ProcessChildren does.
That seems better.
Comment on attachment 809346 [details] [diff] [review] patch [I'm assuming you hadn't seen comment 4 yet when you posted this.] Looks like we need to call PushFloatContainingBlock(nullptr, ...) , basically matching the code here: http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp#9061
(In reply to Daniel Holbert [:dholbert] from comment #6) > [I'm assuming you hadn't seen comment 4 yet when you posted this.] Indeed. This could probably do with a code comment if the reason for this code is easily forgotten, but I will need some help on the wording if I'm to add one.
bz: does it also make sense to add an assertion to PushFloatContainingBlock as dholbert suggested?
Comment on attachment 809352 [details] [diff] [review] patch r=me
Attachment #809352 - Flags: review?(bzbarsky) → review+
Re comment 8: Probably a good idea, yes.
Summary: Move an assertion in nsCSSFrameConstructor::ConstructFrameFromItemInternal into the block to which it applies to avoid bogus failures → Make nsCSSFrameConstructor::ConstructFrameFromItemInternal handle frames that should suppress floating of descendants.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.