29061 - Relative width value of "*" used with COL element produces unexpected results

Status: VERIFIED FIXED

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

Description

[u11984]

As the test case illustrates setting WIDTH to "*" and "1*" in the COL tag
produces different results, which is in breach of HTML 4.01 specification,
section 6.6 (<http://www.w3.org/TR/html4/types.html#type-multi-length>).
I'm running M13 on Windows NT with IE 5.01.

Comment 1

[u11984]

Attachment: Test case for the bug

Comment 2

[Christine Hoffman]

In the testcase, the first table is rendering correctly. In the first table, the 
width of the first column should span 1/4 of the window and the width of the 
second column should span 3/4 of the window. 

The problem occurs when "*" or "0*" relative width values are used. The first 
column should span the minimum width necessary to hold the column's contents - 
which it does. I'm not sure what the behavior for the second column should be - 
I would assume it should span the remainder of the window. If there is only 1 
column with a relative width value of "0*", the column spans the entire window. 
Providing another testcase with 3 examples.

Revising the summary somewhat.

Comment 3

[Christine Hoffman]

Attachment: expanded testcase

Comment 4

[karnaze (gone)]

mass move to m16

Comment 5

[karnaze (gone)]

Moving to M18.

Comment 6

[karnaze (gone)]

Mark, the style system is converting "*" to a proportional value of 0 which is 
accounting for the problem in the 1st attachment. After that is fixed there may 
still be a table problem when dealing with the 2nd attachment. 

Comment 7

[Marc Attinasi]

Actually, it is the GenericHTMLElement that is making * == 0* instead of 1*

The method nsGenericHTMLElement::ParseValueOrPercentOrProportional needs to 
check for the special case of a proportional value '*' and set the value to 1, 
not 0. Easy fix, in my tree, I'll attach the patch. 

How important is this? Does anybody want to nominate it? The (first part of the) 
fix is simple enough, any remainder needs to go back to karnaze.

Comment 8

[Marc Attinasi]

The patch I'm about to attach makes testcase 1 look correct (note that IE and 
Nav4 both get it wrong, differently). The second testcase shows layout problems 
in tables, not a parsing problem in attributes.

Comment 9

[Marc Attinasi]

Attachment: Patch to fix the incorrect parsing of width value '*'

Comment 10

[Marc Attinasi]

Sending back to karnaze to deal with the remaining problems with table layout. 
Use the attached patch to fix the first problem.

Comment 11

[karnaze (gone)]

I'm changing the summary from "Relative width value of "0*" used with COL 
element produces unexpected results" to "Relative width value of "*" used with 
COL element produces unexpected results" and reassigning back to attinasi. I'll 
spawn a new bug to deal with the "0*" issues. 

Marc, I think this bug should make it into FCS because it is low risk.

Comment 12

[karnaze (gone)]

I just filed bug 42033 to deal with "0*"

Comment 13

[Marc Attinasi]

Nominating for nsbeta3: fix is in attached patch and is extremely low risk, 
simple, etc.

Comment 14

[Marc Attinasi]

Setting milestone and updating platform / OS / priority fields.

Comment 15

[Marc Attinasi]

Approved for beta3: low risk, correctness of HTML4.01 impl. and already fixed.

Comment 16

[Marc Attinasi]

Fix checked in. (nsGenericHTMLElement.cpp)

Comment 17

[Christine Hoffman]

Using 8/25 build, bug is not fixed. If you look at the first testcase, the 
second table's col width's are not distributed correctly. It should be identical 
to the first table. 

Reopening bug.

Comment 18

[Marc Attinasi]

nsGenericHTMLElement::ParseValueOrPercentOrProportional has been changed and now 
it parses the '*' and tries to convert it to an integer and gets and error. This 
was part of jst and vidur's big string change, and it doesn;t quite work here.

The fix is simple: allow the error NS_ERROR_ILLEGAL_VALUE since that is actually 
valid in some cases. Patch to be attached...

Comment 19

[Marc Attinasi]

Attachment: New patch for the new string semantic: allow illegal-integer values in ParseValueOrPercentOrProportional

Comment 20

[Marc Attinasi]

Fix checked in (nsGenericHTMLElement.cpp)

Comment 21

[Christine Hoffman]

Tested across platform on 9/7 builds. Verifying bug fixed in regards to the "*" 
issue. Acknowledge that problems with "0*" still exist and have been written up 
in bug #42033.