Closed Bug 330970 Opened 18 years ago Closed 18 years ago

Crash with absolute positioned object and setting display table on documentElement

Categories

(Core :: Layout: Tables, defect)

Product:
Core
Component:
Layout: Tables
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: martijn.martijn, Unassigned)

References

Details

(Keywords: crash, regression, testcase)

Attachments

(4 files)

testcase
451 bytes, text/html
Details
Backtrace
28.04 KB, text/plain
Details
reduced tectase that triggers assertion
250 bytes, text/html
Details
patch
5.46 KB, patch
bzbarsky
: review-
Details | Diff | Splinter Review 
See upcoming testcase, which crashes current trunk Mozilla build.
It doesn't crash always, if it doesn't crash directly, try clicking on the 'Go' button a few times.

Talkback ID: TB16547758Z
nsTableOuterFrame::RemoveFrame   nsCSSFrameConstructor::RecreateFramesForContent   nsCSSFrameConstructor::RestyleElement   0x02031790
0x0f6375c0

Doesn't seem to crash in 2005-09-21 build, crashes with 2005-09-22 build. Probably changed with bug 1156.
Attached file testcase — 

    
    

    
  

      
      
      
      

        Attached file
          
        Backtrace
        — 
      

    


  
Backtrace from debug build.
I get a couple of assertions, prior to the crash (stacks included for the first two):
###!!! ASSERTION: wrong childlist: '!aListName', file c:/mozilla/mozilla/layout/
tables/nsTableOuterFrame.cpp, line 195

###!!! ASSERTION: Frame leak!: 'mFrames.IsEmpty()', file c:/mozilla/mozilla/layo
ut/tables/nsTableOuterFrame.cpp, line 196

###!!! ASSERTION: incomplete children: 'PR_FALSE', file c:/mozilla/mozilla/layou
t/tables/nsTableOuterFrame.cpp, line 1856

###!!! ASSERTION: can't remove inner frame: 'nsLayoutAtoms::captionList == aList
Name', file c:/mozilla/mozilla/layout/tables/nsTableOuterFrame.cpp, line 258

And the crash itself:
#0  0x05cc82e8 in nsIFrame::AddStateBits(unsigned) (this=0x0, aBits=1024)
    at c:/mozilla/mozilla/layout/generic/nsIFrame.h:901
#1  0x05886d21 in nsTableOuterFrame::RemoveFrame(nsIAtom*, nsIFrame*) (
    this=0xdccd3e0, aListName=0xe94528, aOldFrame=0xe72bb8c)
    at c:/mozilla/mozilla/layout/tables/nsTableOuterFrame.cpp:268
#2  0x056c591e in nsFrameManager::RemoveFrame(nsIFrame*, nsIAtom*, nsIFrame*)
    (this=0xe722344, aParentFrame=0xdccd3e0, aListName=0xe94528,
    aOldFrame=0xe72bb8c)
    at c:/mozilla/mozilla/layout/base/nsFrameManager.cpp:682
#3  0x0569619a in nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*
, int, int) (this=0xe6d6208, aContainer=0xe5b7ca8, aChild=0xe72b208,
    aIndexInContainer=1, aInReinsertContent=0)
    at c:/mozilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:10112
etc...

    
    

    
  
this uses abs. pos marking a guessed dependency on bug 330909
Depends on: 330909
Flags: blocking1.9a1?

    
    

    
  
<html style="display:table">
 <body >    
  <div style="position: absolute;"></div>
 </body>
</html>

On the first chunk of frame creation we create a outer table frame within ConstructDocElementTableFrame

and then try to attach the absolute items inside ProcessFrameInsertions. Tables cannot handle the named childlist, happily assert it and return a failure.

Then bz's comment realizes:

  // XXXbz And if NS_FAILED(rv), what?  I guess we need to clean up the list
  // and deal with all the placeholders... but what if the placeholders aren't
  // in the document yet?  Could that happen?
  NS_ASSERTION(NS_SUCCEEDED(rv), "Frames getting lost!");

Once the frames are lost, it takes only a couple of dom mutations till we crash.

    
    

    
  
Comment 3 is garbage, imho
No longer depends on: 330909

    
  
Blocks: 340946

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch
        — 
          — Splinter Review
      

    


  
Boris, this fixes the crash and all asserts. I am however not confident that it the right thing to do. Bug 320865 hints to that but I might have it misunderstood.

So this is more a plea for advice, if this the way to go then I get the patch into trunk shape.

        Attachment #239875 -
        Flags: review?(bzbarsky)

    
    

    
  
Mmm... I'm not actually sure what the right thing to do is here.  For fixed pos we definitely want something different from this; not sure about abs pos.  In particular, the question is how things should behave given margins on the table.

In this testcase, isn't the problem simply that GetAbsoluteContainingBlock() is busted?  I think comment 3 was right on the money...

> and then try to attach the absolute items inside ProcessFrameInsertions. 

This is the bustage caused by GetAbsoluteContainingBlock().  There should be no absolute items, for the time being, until tables can support abs pos lists.
Depends on: 330909

    
    

    
  
Land the reflow branch and I promise to make the latter happen!

    
    

    
  
OK, so with bug 330909 fixed, is this still an issue?

    
    

    
  
(In reply to comment #10)
> OK, so with bug 330909 fixed, is this still an issue?

No, the 2006-09-25 trunk build doesn't crash anymore on the testcase -> fixed by bug 330909.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED

        Attachment #239875 -
        Flags: review?(bzbarsky) → review-
Flags: blocking1.9a1?

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: