308019 - ###!!! ASSERTION: Computed overflow area must contain frame bounds: 'aNewSize.width == 0 || aNewSize.height == 0 || aOverflowArea->Contains(nsRect(nsPoint(0, 0), aNewSize))', file nsFrame.cpp, line 4301

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Mats Palmgren (inactive)]

###!!! ASSERTION: Computed overflow area must contain frame bounds:
'aNewSize.width == 0 || aNewSize.height == 0 ||
aOverflowArea->Contains(nsRect(nsPoint(0, 0), aNewSize))', file nsFrame.cpp,
line 4301


STEPS TO REPRODUCE
1. Load URL

Comment 1

[Mats Palmgren (inactive)]

Attachment: stack + data + frames

Comment 2

[Boris Zbarsky [:bzbarsky]]

Martijn, do you think you can testcase this?

Comment 3

[Martijn Wargers (dead)]

Attachment: testcase

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: I think this is the right fix...

Comment 5

[Bernd]

>I think this is the right fix...
as in this is not wallpaper?

I have no problem to r+ this but I am not convinced that we are not only wallpapering about some issues here. At least the reflow log looks suspicious to me

 tblO 037434BC r=0 a=13995,UC c=0,UC cnt=162
  tbl 03743624 r=0 a=13995,UC c=UC,UC cnt=163
   rowG 0373F064 r=0 a=UC,UC c=UC,UC cnt=164
    row 0374396C r=0 a=UC,UC c=UC,UC cnt=165
     cell 03743BF4 r=0 a=UC,UC c=UC,UC cnt=166
      block 03743C54 r=0 a=UC,UC c=UC,UC cnt=167
       text 03743DFC r=0 a=UC,UC c=UC,UC cnt=168
       text 03743DFC d=0,0 me=0
       tblO 03743F08 r=0 a=UC,UC c=UC,UC cnt=169
        tbl 03750120 r=0 a=UC,UC c=UC,UC cnt=170
        tbl 03750120 d=88,60 me=88  <<< pixel alignment, how do we get this
       tblO 03743F08 d=88,60 me=88
       text 03750B4C r=0 a=UC,UC c=UC,UC cnt=171
       text 03750B4C d=0,0 me=0
      block 03743C54 d=88,60 me=88 
     cell 03743BF4 d=120,90 me=120
    row 0374396C d=UC,90
   rowG 0373F064 d=UC,90
   colG 03743A38 r=0 a=UC,UC c=UC,UC cnt=172
    col 03750CBC r=0 a=0,0 c=0,UC cnt=173
    col 03750CBC d=0,0
   colG 03743A38 d=0,0
   rowG 0373F064 r=2 a=120,UC c=120,UC cnt=174
    row 0374396C r=2 a=120,UC c=120,UC cnt=175
     cell 03743BF4 r=2 a=120,UC c=90,UC cnt=176
      block 03743C54 r=2 a=90,UC c=90,UC cnt=177
       text 03743DFC r=2 a=90,UC c=UC,UC cnt=178
       text 03743DFC d=0,0 me=0
       tblO 03743F08 r=2 a=90,UC c=0,UC cnt=179
        tbl 03750120 r=2 a=90,UC c=90,UC cnt=180
         rowG 0374380C r=2 a=30,UC c=30,UC cnt=181
          row 03750368 r=0 a=30,UC c=30,UC cnt=182
           cell 037505D8 r=0 a=0,UC c=0,UC cnt=183
            block 03750638 r=0 a=-30,UC c=0,UC cnt=184

Comment 6

[Boris Zbarsky [:bzbarsky]]

> as in this is not wallpaper?

That's what I was thinking.  But I'm not sure.  ;)

The real issue is just an avail size that's too small for our paddings and border, right?  That can Just Happen....

Comment 7

[Boris Zbarsky [:bzbarsky]]

Bernd, see comment 6?

Comment 8

[Bernd]

Boris: this is more a review delay warning, I would like to first make sure that I understand what is the bug behind this, before I sign it.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Oh, that's ok.  This is not exactly blocking anything else I'm working on.  ;)

Comment 10

[Bernd]

Attachment: this fixes the problem where it starts

The problem occures in autowidth cells/cols in fixed layout. The min width should be 0 + border padding and not 0.

The problem before that I see is more should this code really invoke fixed layout?
The content width  of the outer table becomes dependent of the pct inner table. If I read the CSS 2.1 such a pct spec is illegal. Shouldn't we treat the inner table then as auto?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Hmm... yeah, in this case the inner table should indeed be auto.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

What patch do we want here? Both?

Comment 13

[Boris Zbarsky [:bzbarsky]]

I suspect we just want bernd's patch, but check with him?  I really don't know this code very well.  :(

Comment 14

[Bernd]

>I really don't know this code very well.  :(

I like understatement...

If we take Boris patch we will plug the hole completely, we will not trigger the assert over this path. However even trying to pass a negative width usually indicates that something in the layout strategy went wrong. So it would make sense to assert if we clip. But this would otherwise assert anyway with the overflow assert. We hit the assert seldom. I think I can  handle the load from the overflow assert without clipping.

My patch needs to be rtested. Thats why I did not ask for approval

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 209312 [details] [diff] [review]
I think this is the right fix...

So assuming bernd's patch passes the rtests we should take that one.

Comment 16

[Bernd]

Comment on attachment 209486 [details] [diff] [review]
this fixes the problem where it starts

I just rtested it.

Comment 17

[Bernd]

fix checked in