Closed
Bug 386142
Opened 17 years ago
Closed 4 years ago
fantasai's li'l Need More Comments bug
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
INACTIVE
People
(Reporter: fantasai.bugs, Assigned: fantasai.bugs)
Details
Attachments
(3 files, 1 obsolete file)
19.30 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
1.04 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
3.19 KB,
patch
|
cajbir
:
review+
|
Details | Diff | Splinter Review |
Splitting out some comment fixes from bug 379349.
Well, there's a couple things that aren't quite comment/whitespace fixes. One removal of an unused variable, one removal of a duplicated 3 lines of code.... I forgot the context on that, here it is: // If we have a next-in-flow, then we're not complete // XXXldb This used to be done only for the incremental reflow codepath. if (GetNextInFlow()) { aStatus = NS_FRAME_NOT_COMPLETE; } SetHasStyleHeight((NS_UNCONSTRAINEDSIZE != aReflowState.mComputedHeight) && (aReflowState.mComputedHeight > 0)); // just set our width to what was available. The table will calculate the width and not use our value. aDesiredSize.width = aReflowState.availableWidth; // if we have a nextinflow we are not complete if (GetNextInFlow()) { aStatus |= NS_FRAME_NOT_COMPLETE; } aDesiredSize.mOverflowArea.UnionRect(aDesiredSize.mOverflowArea, nsRect(0, 0, aDesiredSize.width,
Attachment #270120 -
Attachment is obsolete: true
Attachment #270122 -
Flags: review?(roc)
Attachment #270120 -
Flags: review?(roc)
Comment 4•17 years ago
|
||
> - NS_ASSERTION(mFrameCount == 0, "Some frame destructors were not called"); > + NS_ASSERTION(mFrameCount == 0, "Some frame or linebox destructors were not called"); While you're trying to make my assertion message from bug 334514 more accurate, you might be interested in bug 365909 comment 5 and bug 365909 comment 15.
Attachment #270122 -
Flags: superreview+
Attachment #270122 -
Flags: review?(roc)
Attachment #270122 -
Flags: review+
In that case, I suggest "Some frames or other AllocateFrame'd objects not destroyed"
Comment 6•17 years ago
|
||
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Comment on attachment 270122 [details] [diff] [review] patch A (with context) >Index: layout/base/nsPresShell.cpp >- NS_ASSERTION(mFrameCount == 0, "Some frame destructors were not called"); >+ NS_ASSERTION(mFrameCount == 0, "Some frame or linebox destructors were not called"); A bunch of other things are allocated in the frame arena. It would be clearer not to mention line boxes specifically, but make it more general.
Comment 8•17 years ago
|
||
(In reply to comment #7) > (From update of attachment 270122 [details] [diff] [review]) > >Index: layout/base/nsPresShell.cpp > >- NS_ASSERTION(mFrameCount == 0, "Some frame destructors were not called"); > >+ NS_ASSERTION(mFrameCount == 0, "Some frame or linebox destructors were not called"); > > A bunch of other things are allocated in the frame arena. It would be clearer > not to mention line boxes specifically, but make it more general. I generalized it to "Some objects allocated with AllocateFrame were not freed" when I checked it in.
Couldn't find anything about an AppendToFlow function, but I did check that nsSplittableFrame::Init sets up both sides of the two-way link.
Attachment #272850 -
Flags: superreview?(roc)
Attachment #272850 -
Flags: review?(roc)
Attachment #272850 -
Flags: superreview?(roc)
Attachment #272850 -
Flags: superreview+
Attachment #272850 -
Flags: review?(roc)
Attachment #272850 -
Flags: review+
Keywords: checkin-needed
Whiteboard: checkin needed for patch to nsIFrame.h::Init comments
Comment 10•17 years ago
|
||
nsIFrame::Init patch committed.
Keywords: checkin-needed
Whiteboard: checkin needed for patch to nsIFrame.h::Init comments
Assignee | ||
Comment 11•15 years ago
|
||
Attachment #404391 -
Flags: review?(chris.double)
Attachment #404391 -
Attachment is patch: true
Attachment #404391 -
Attachment mime type: application/octet-stream → text/plain
Updated•15 years ago
|
Attachment #404391 -
Flags: review?(chris.double) → review+
Assignee | ||
Comment 12•15 years ago
|
||
carriage returns patch pushed to mozilla-central http://hg.mozilla.org/mozilla-central/rev/90d2c483697b
Comment 13•4 years ago
|
||
It looks like all the patches landed, or at least two of the three.
Surely anything that hasn't landed is well obsolete?
Flags: needinfo?(vseerror)
Updated•4 years ago
|
Status: REOPENED → RESOLVED
Closed: 17 years ago → 4 years ago
Flags: needinfo?(svoisen)
Resolution: --- → INACTIVE
comment 6, comment 10, and comment 12 makes it look like all three
You need to log in
before you can comment on or make changes to this bug.
Description
•