194014 - abs. pos. div too tight for auto width table

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect, P4)

Description

[Olivier Borowski]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030210
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030210

This problem occurs in Mozilla 1.3b & nightly builds :
A layer (no width, no height) contains a table. The width of the table is
defined only by the <td> tags. When we try to fill the last cell with a picture
larger than it, the width of other cells decrease. The table width should
probably increase instead! (it is what happend in any other browser e.g.
mozilla<1.3b ...)

Reproducible: Always

Steps to Reproduce:
1. a layer (no width, no height) contains a table
2. the table has 1 line and some cols
3. cols width is indicated with <td> tags (except the last column)
4. the last cell contains a picture (empty or small width)
5. this picture is exchanged with a wider picture
Actual Results:  
The last cell takes the width of the picture (normal) but others colomns become
smaller

Expected Results:  
The whole table should become wider

Comment 1

[Markus Hübner]

confirming - trunk build 2003021608 on winxp pro sp1

Comment 2

[Vincent Nicolas]

Again, it is a problem on first load. Doing a Crtl-R (reload) makes the display
right. It is may be linked to bugs :
http://bugzilla.mozilla.org/show_bug.cgi?id=146851
http://bugzilla.mozilla.org/show_bug.cgi?id=156629
http://bugzilla.mozilla.org/show_bug.cgi?id=171325
http://bugzilla.mozilla.org/show_bug.cgi?id=178430
etc... ?

Comment 3

[Bernd]

Attachment: reduced testcase

Comment 4

[Bernd]

Attachment: more obvious testcase

Comment 5

[Bernd]

Arunan has this something to do with your checkin for bug 97777 ?

Comment 6

[Arunan Balasubramaniam]

Yes, removing those two lines fixes this :(

Comment 7

[Bernd]

changing component

Comment 8

[Markus Hübner]

thx bernd for the investigation.
what are we gonna do for 1.3 - backing out bug 97777 or adapting the patch in 
there?

Comment 9

[Arunan Balasubramaniam]

Is 1.3b really close?  I think I can see where the problem may be in the reflow
log.  On the second reflow (after the image has loaded) you see:

VP 019D7378 r=1 a=9180,4470 c=9180,4470 cnt=59 
 scroll 019D772C r=1 a=9180,4470 c=9180,4470 cnt=60 
 scroll 019D772C r=1 a=9180,4470 c=9180,4470 cnt=61 
  canvas 019D7484 r=1 a=9180,UC c=9180,4470 cnt=62 
   area 01A00318 r=1 a=9180,UC c=9180,UC cnt=63 
    area 01A00840 r=1 a=9180,UC c=UC,UC cnt=64 
     tblO 01A00A38 r=1 a=UC,UC c=0,0 cnt=65 
      tbl 01A00BAC r=1 a=UC,UC c=UC,UC cnt=66 
                         ^^
       rowG 01A00D8C r=1 a=1500,UC c=1500,UC cnt=67 
                           ^^^^
        row 01A00F38 r=1 a=1500,UC c=1500,UC cnt=68 
         cell 01A19664 r=1 a=0,UC c=0,UC cnt=69 
          block 01A19704 r=1 a=0,UC c=0,UC cnt=70 
           img 01A199DC r=4 a=UC,UC c=UC,UC cnt=71 
           img 01A199DC d=9000,870 me=9000 
           img 01A199DC r=2 a=0,UC c=UC,UC cnt=72 
           img 01A199DC d=9000,870 
          block 01A19704 d=9000,938 me=9000 m=9000 
         cell 01A19664 d=9030,975 me=9030 m=9030 
                         ^^^^
        row 01A00F38 d=1500,975 
                       ^^^^
       rowG 01A00D8C d=1500,975 
      tbl 01A00BAC d=1530,1005 
     tblO 01A00A38 d=1530,1005 

  I think that the table is caching the old value of the row's width instead of
recalculating it (this log was with 97777 applied).  1500 is the RowG's width
from the first reflow.  This would explain why the reflow works OK if the image
is in the cache as the row is calculated with the correct width to begin with.

Comment 10

[Arunan Balasubramaniam]

Think I'm jumping to conclusions, sorry.  Got my incremental and initial reflows
mixed up.

Comment 11

[Bernd]

Attachment: testcase that fails on reload

Comment 12

[Arunan Balasubramaniam]

Attachment: Reflow log with 97777.

Comment 13

[Arunan Balasubramaniam]

Attachment: Reflow log without 97777.

Comment 14

[Arunan Balasubramaniam]

Diffing the two logs, when the fix for 97777 is in the table seems to skip an
extra resize reflow and return an incorrect desired size (the same value as
before the image) and then gets reflowed with that size.

Comment 15

[Bernd]

The major difference I see is that the table now gets an unconstrained
incremental reflow:

old:
VP 03B674D0 r=1 a=17475,12660 c=17475,12660 cnt=2002 
 scroll 03B67884 r=1 a=17475,12660 c=17475,12660 cnt=2003 
 scroll 03B67884 r=1 a=17475,12660 c=17475,12660 cnt=2004 
  canvas 03B675DC r=1 a=17475,UC c=17475,12660 cnt=2005 
   area 03BC9730 r=1 a=17475,UC c=17475,UC cnt=2006 
    area 03BCC2D8 r=1 a=17475,UC c=UC,UC cnt=2007 
     tblO 03BCC4D0 r=1 a=17325,UC c=0,0 cnt=2008 
      tbl 03BCC644 r=1 a=17325,UC c=UC,UC cnt=2009 

new:
VP 03B7B180 r=1 a=17475,12660 c=17475,12660 cnt=1983 
 scroll 03B7B534 r=1 a=17475,12660 c=17475,12660 cnt=1984 
 scroll 03B7B534 r=1 a=17475,12660 c=17475,12660 cnt=1985 
  canvas 03B7B28C r=1 a=17475,UC c=17475,12660 cnt=1986 
   area 03C9F290 r=1 a=17475,UC c=17475,UC cnt=1987 
    area 03CA1E58 r=1 a=17475,UC c=UC,UC cnt=1988 
     tblO 03CA2050 r=1 a=UC,UC c=0,0 cnt=1989 
      tbl 03CA21C4 r=1 a=UC,UC c=UC,UC cnt=1990 

Which causes different behaviour at
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableFrame.cpp#2047

Comment 16

[Bernd]

area 03CA1E58 r=1 a=17475,UC c=UC,UC cnt=1988 
 tblO 03CA2050 r=1 a=UC,UC c=0,0 cnt=1989 

I believe this is illegal, you can't (or at least should not) combine an incr.
reflow and a resize reflow. Table frames are not able to handle this and
probably will not be able to do this in any foreseeable future. You should not
alter the available size for an incr. reflow. If you wish to resize make it
explicit and make a resize reflow.

Comment 17

[Bernd]

Attachment: patch

Comment 18

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

Will this still compute the sizes of things correctly when they are
shrink-wrapped and incrementally reflowed?

Comment 19

[Bernd]

The patch renders the testcases in bug 97777 fine and and stops the table
bustage.  And I stand by my comment merging incr. and resize reflows is bad.

Comment 20

[Greg K.]

*** Bug 195071 has been marked as a duplicate of this bug. ***

Comment 21

[Greg K.]

Setting All/All per comment 20.

Comment 22

[Bernd]

Comment on attachment 115644 [details] [diff] [review]
patch

the patch does not fix the dupe bug 195071. The only thing that really helps is
to backout the patch for bug 97777. What happens here is that shrink wrap
mechanisms collide. I don't know how to solve this. So we either live with
horked tables ( but nesting them in abs. pos. is the recommended workaround for
problems with auto abs. pos. tables) or we  regress the divs. But thats more a
drivers decision than mine.

Comment 23

[Bernd]

Attachment: patch

Comment 24

[Bernd]

Comment on attachment 115895 [details] [diff] [review]
patch

This patch is a backout of the patch for bug 97777. The patch for bug 97777 has
been checked in despite that the fact that it breaks the layout regression
tests. I don't understand why we check in testcases if the warning signs from
broken regression testcases are simply ignored. Neither can I accept such an
approach. I aim to get this in also or 1.3 in order to prevent regression bug
reports. The behaviour introduced with bug 97777 is different from IE, Moz and
NN up to now, so there will certainly be those reports. 
Due to bug 97777 not only auto but also percent tables are broken in an abs.
pos. div.

Comment 25

[Arunan Balasubramaniam]

Apologies Bernd.  I thought I had run and interpreted the regression tests
correctly, but obviously had not.

Comment 26

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

I agree that the patch should be backed out for 1.3.

Do we actually have a test for these cases in the layout regression tests?

Comment 27

[Asa Dotzler [:asa]]

Comment on attachment 115895 [details] [diff] [review]
patch

a=asa (on behalf of drivers) for checkin to the 1.3 branch.

Comment 28

[Bernd]

fix checked in (trunk and branch)

Comment 29

[Arunan Balasubramaniam]

*** Bug 194586 has been marked as a duplicate of this bug. ***