superreview requested: [Bug 341382] [FIX] Crash [@ DoDeletingFrameSubtree] with position:fixed and display: table-caption : [Attachment 227367] Patch rev. 2 (diff -w)

Mats Palmgren <mats.palmgren@bredband.net> has asked Boris Zbarsky (gone June
26 -- Jul 13) <bzbarsky@mit.edu> for superreview:
Bug 341382: [FIX] Crash [@ DoDeletingFrameSubtree] with position:fixed and
display: table-caption
https://bugzilla.mozilla.org/show_bug.cgi?id=341382

Attachment 227367: Patch rev. 2 (diff -w)
https://bugzilla.mozilla.org/attachment.cgi?id=227367&action=edit

------- Additional Comments from Mats Palmgren <mats.palmgren@bredband.net>
(In reply to comment #9)
> Why the !aParent check for the next sibling?

Many call nsFrameList::InsertFrames() with a null aParent to avoid the
parent assignment at the beginning, I guess because that is already done
in most cases.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsFrameList
..cpp&rev=3.40&root=/cvsroot&mark=194-200#187

I changed it to test aFrameList->GetParent() instead which should
cover both cases. I also added the same assertions to
nsFrameList::InsertFrame() and moved the parent assignment earlier
(this method is immideately above InsertFrames() in the link above)
There are no other changes compared to rev. 1.

> 
> >Index: layout/tables/nsTableFrame.cpp
> > nsTableFrame::RemoveFrame(nsIAtom*	      aListName,
> >   if (NS_STYLE_DISPLAY_TABLE_COLUMN_GROUP == display->mDisplay) {
> >+	NS_ASSERTION(!aListName || aListName == nsLayoutAtoms::colGroupList,
> >+		     "unexpected child list");
> 
> Can we just assert aListName == nsLayoutAtoms::colGroupList here?  We're
> certainly assuming that mColGroups is the thing to mess with...

I tried that at first but got assertions since the frame constructor removes
col-group frames using nsnull in some cases... when I read the code I got the
impression this is by design.
So I decided to be lenient in this case and accept nsnull or colGroupList but
not anything else. (sorry, I should have explained this before)
0
bugzilla
6/28/2006 5:49:35 AM
mozilla.dev.super-review 29307 articles. 3 followers. Post Follow

0 Replies
695 Views

Similar Articles

[PageSpeed] 42

Reply:

Web resources about - superreview requested: [Bug 341382] [FIX] Crash [@ DoDeletingFrameSubtree] with position:fixed and display: table-caption : [Attachment 227367] Patch rev. 2 (diff -w) - mozilla.dev.super-review

Resources last updated: 12/25/2015 1:10:48 AM