35662 - Incorrect response to <td width=x%> on two rows

Status: VERIFIED FIXED

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

Description

[Phil "clef" Sweeney]

(This is probably all platforms but I haven't checked)

Builds: All nightly builds for the last few weeks (as far as I can see)

Below is a simple testcase that has 2 small tables.

Expected: They should both have the same overall table width (this is the case 
with Nav 4.7 and IE5)

Result: In mozilla, the second table has a bigger overall width.

<table border=1>
    <tr>
        <td>Left side</td>
        <td width=20%>Mid</td>
        <td>Right side</td>
    </tr>
</table>

<table border=1>
    <tr>
        <td>Left side</td>
        <td width=20%>Mid</td>
        <td>Right side</td>
    </tr>
    <tr>
        <td>Left side</td>
        <td width=20%>Mid</td>
        <td>Right side</td>
    </tr>
</table>

Comment 1

[Phil "clef" Sweeney]

Setting testcase keyword

Comment 2

[dark]

looks like a duplicate of bug 27555

Comment 3

[troy]

Changing component

Comment 4

[Doron Rosenberg (IBM)]

confirming bug

Comment 5

[Phil "clef" Sweeney]

I think this should be considered for a higher priority.  This very simple HTML 
could cause many pages to render incorrectly.

Comment 6

[karnaze (gone)]

Ok, I'll mark it M17. 

Comment 7

[Phil "clef" Sweeney]

I see this isn't going to get touched for a while.  Karnaze could you tell me 
which part of the code this is in.. I don't claim to have any experience coding 
in Mozilla, but I might have a look anyway.  Thanks.

Comment 8

[Doron Rosenberg (IBM)]

Attachment: testcase as reporter has in bug description

Comment 9

[Jacob Kjome]

Sorry, but considering whether to fix this or not for RTM is a no-brainer.  This
has to be done.  I've seen TONS of pages use this sort of coding.  Like it or
not, it is valid HTML and ought to be rendered right.

Sending this sort of bug into the browser market is going to start the whole
workaround-the-browser-bugs crap all over again.  It is bad enough with
javascript.

Please fix this!!!

jake

Comment 10

[Hixie (not reading bugmail)]

hoju: Could you give us some sites that do indeed use this markup and therefore
render incorrectly? There are lots of 'no brainer this must be fixed' bugs,
but in the end if you fix all of them you introduce lots of regressions. Fixing
a bug doesn't just fix it, it also runs the risk of breaking something else.
If you could show us how common this is, we could make a better-informed
decision. Thanks!

Comment 11

[Jacob Kjome]

Hmmmm... real world examples, heh?  Can't think of any specifically.  My 
statement was based on seeing code like this on sites that our company has 
created over the years.  Most aren't public.  

My question to you is, why do you need an actual real life example?  Seems like 
this is pretty simple table behavior to me and the testcase shows that it 
doesn't work.  I believe it works in every browser out there but this one which 
is supposed to be the "most standards compliant browser ever".

I suppose the argument is "If this doesn't affect too many people, then just 
put it off".  That reasoning would fly except for the fact that this is basic 
table layout stuff and the testcase is valid HTML.  If I want to code the way 
the testcase is set up, I ought to be able to with no worries about which 
browsers will render it correctly.

My argument is based more on principal than on real world examples.  It just 
seem that with the number of table rendering bugs in this browser, the RTM date 
would be pushed back.  I just want an awesome browser that works correctly and 
I'm willing to wait for it.  Don't pass over stuff that is important just to 
meet a deadline.

Jake

Comment 12

[Hixie (not reading bugmail)]

Hey, I agree entirely. I'm sure it will be fixed for Mozilla 1.0, too! However,
Netscape have some committement to release a browser soon, and so will be 
releasing it based on a pre-release version of Mozilla, so unless someone can
show us that this is very important, it is unlikely that anyone will get to 
look at this bug until after Netscape have released their product.

If you think this is misguided, the people to speak to are [e-mail] (the
Netscape Product Development Team).

Comment 13

[Doron Rosenberg (IBM)]

nomianting for 0.9

Comment 14

[Bernd]

The offending codeline is:

http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/BasicTableLayoutStrategy.cpp#1039

The percent width is added up over the rows instead of taking the maximum for a
given column over all rows.

Comment 15

[Phil "clef" Sweeney]

Attachment: Proposed patch to fix required cell width

Comment 16

[Phil "clef" Sweeney]

I have attached a patch that fixes this issue.  It works with the testcase and 
also with a webpage exhibiting this issue 
(http://yallara.cs.rmit.edu.au/~izzo/)  Of course it may cause regressions 
elsewhere so I would appreciate it if people could test it.  This is my first 
patch for Mozilla, and it's a small one :)  ..  thanks to Bernd Mielke for 
identifying the problem with the code!

Comment 17

[timeless]

It looks harmless. karnaze would you review this and help clef get it or a 
derivative committed? thanks clef, karnaze

Comment 18

[karnaze (gone)]

Attachment: patch which handles more cases

Comment 19

[Chris Waterson]

sr=waterson

Comment 20

[karnaze (gone)]

The patch is in.

Comment 21

[Phil "clef" Sweeney]

This appears fixed in the nightlies now.  Thanks to Bernd Mielke and Karnaze for 
their work.