46944 - when using percentage width table layout differs from NN47 and IE

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect, P3)

Description

[Devoti Paolo]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; m17) Gecko/20000729
BuildID:    2000072904

the layout of the table is different from NN47 and is likely to be wrong in Mozilla

Reproducible: Always
Steps to Reproduce:
1.open the provided url or the attached simplified page



Actual Results:  Both NN47 and mozilla are displaing the page but the layout is
dramatically different

Expected Results:  same layout

Comment 1

[Devoti Paolo]

Attachment: simplified page showing the effect

Comment 2

[Ari Pollak]

it doesn't really look like it's incorrect, even thought it's different - the
HTML is very screwy, and doesn't specifically say a fixed size for the columns.

Comment 3

[Devoti Paolo]

The layout I get from Mozilla is a two columns table, the left one is larger
then the right one, with NN47 the right one is the larger.
IMO the witdh=20% is setting the width of the left column to 20% of the current
margins so the NN47 behaviour is the correct one. IE5 is doing like NN47.

Comment 4

[Mats Palmgren (inactive)]

Attachment: even more simplified testcase

Comment 5

[Peter 'Rufus' Nelson]

Confirming

Comment 6

[Peter Lubczynski]

Even though the markup is screwy, we should still do this correctly for cross 
browser compatibility. IE and Nav do this correctly.

Looking at the second test case (12121), using a fixed or auto width for the 
first row with the colspan solves this problem. Percentage width is not being 
calculated correctly and is throwing off the whole table. 

Comment 7

[Devoti Paolo]

Still real with (win98 2000111720)

Comment 8

[Devoti Paolo]

Still real with win98 2001031604

Comment 9

[Amarendra Hanumanula]

QA contact update

Comment 10

[Bernd]

steeling this bug

Comment 11

[Bernd]

Analysis: In the attached testcase a colspan requires only 20% while covering
the all columns.  Currently we take the authors intentions as real compute the
numbers based on the wrong assumption of 20% width and later grow the spanned
columns wrong. 
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/BasicTableLayoutStrategy.cpp#1529

My guess is that IE5 and NN4 simply ignore the percent width for these colspans.
We could do the same by assuming a 100% width for this colspan.

see the following patch

@@ -1527,6 +1530,8 @@
       float cellPct = 0.0f;
       if (eStyleUnit_Percent == cellPosition->mWidth.GetUnit()) {
         cellPct = cellPosition->mWidth.GetPercentValue();
+        if(colSpan == numEffCols)
+           cellPct=1.0f;
         cellPctWidth = nsTableFrame::RoundToPixel(NSToCoordRound(
((float)basis) * cellPct ), aPixelToTwips);
         if (!mIsNavQuirksMode) {
           // need to add padding

The patch passes the table regression tests

Comment 12

[karnaze (gone)]

r=karnaze. Bernd, please make whitespace consistent with the rest of the code,
please make an attachment out of the patch. Thanks.

Comment 13

[Bernd]

Attachment: patch with right white spaces (I hope)

Comment 14

[Marc Attinasi]

Looks good to me. sr=attinasi  Thanks again Bernd!

Comment 15

[Bernd]

fix + testcase checked in

Comment 16

[Amarendra Hanumanula]

 I dont see any differences in the testcase between NS6.1 and NS4.77, IE 5.0
Platform : win98
Build ID# 2001060409
marking verified